-
Notifications
You must be signed in to change notification settings - Fork 63
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
elbepack: hdimg: enable passing environment variables to grub #432
Conversation
The update-grub2 script is customizable by passing environment variables; however, it was impossible to do so in Elbe. This commit enables passing environment variables to the update-grub2 script in addition to the additional parameters for install-grub. Every word containing an equal sign will be interpreted as an environment variable. Signed-off-by: Thomas Bonnefille <[email protected]>
8f62b91
to
e044197
Compare
@@ -154,6 +154,11 @@ def install(self, target, user_args): | |||
if '/' not in self.fs: | |||
return | |||
|
|||
# Extract environnement variable from user_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in "environment"
Installs grub on this harddisk. The text content will be split | ||
between additional command line arguments given to Elbe's | ||
grub-install call and environment variables will be given to | ||
update-grub2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really bad interface. Also it only works for grub2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'm aware that the interface is not really good but as update-grub
can be fed with environment variable, I thought that it was a good idea to have this kind of feature.
Installs grub on this harddisk. The text content will be split | ||
between additional command line arguments given to Elbe's | ||
grub-install call and environment variables will be given to | ||
update-grub2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For new features also create an entry in newsfragments/
.
Installs grub on this harddisk. The text content will be split | ||
between additional command line arguments given to Elbe's | ||
grub-install call and environment variables will be given to | ||
update-grub2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if these parameters should be come from the ELBE XML in the first place.
If update-grub2
is executed again manually or from apt/dpkg hooks, the configuration is not used anymore.
Instead if the configuration is written to /etc/default/grub
or /etc/default/grub.d/
in the target it would always be respected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the same thing can be said about the current situation where we can pass parameters to grub-install
. The goal of having it in the XML is to easily setup a specific way of setting up grub2 without having to hack into the rootfs with an overlay.
However indeed the user interface is not the best here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The config files could also be written through finetuning. Then the contents are directly in the ELBE XML.
It will also handle grub updates in the field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so do you think I should persevere on this PR or is using finetuning/overlay sufficient for advanced users ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If finetuning/overlay does indeed work for you I would do prefer sticking to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, it indeed work in that way but I feel like I'm hacking into the rootfs instead of using the features provided by update-grub.
But no problem :), I'll close this PR, thank you very much for your reviews !
The update-grub2 script is customizable by passing environment variables; however, it was impossible to do so in Elbe.
This commit enables passing environment variables to the update-grub2 script in addition to the additional parameters for install-grub.
Every word containing an equal sign will be interpreted as an environment variable.
This is particularly useful for example when you want to force Grub to use PARTUUID (mandatory when booting without an initramfs).