Re: Gfxboot theme for booting
On Tue, Feb 24, 2009 at 5:30 AM, Daniel Baumann <daniel@debian.org> wrote:
> Juanje Ojeda Croissier wrote:
>> Hi guys! :-)
>
> Hi,
Hi again and thanks for your comments :-)
>> We hope you find some of those useful and if you prefer any way we give
>> to you our patches or features, just let us know.
>
> sending them to the mailinglist is just fine, especially if you want to
> discuss them. if you also mention where they can be fetched trough git,
> that's nice as well and makes merging faster. for that one, i already
> know that it's on github (i guess) from where i've got the last patch.
Yes, we have some branches on github. We are not git expert but we're
trying to keep updated our branches with yours to be more compatible
and easy to integrate.
Roberto (which I think is already in this list) has the branch in
where is depeloping the features here:
http://github.com/rcmorano/live-helper-guadalinex/tree/master
And I have two branches (one is a mirror of yours and the other
integrates the changes from Roberto with the mirror) inhere:
http://github.com/juanje/live-helper/tree/master
Now there are more features we are playing with on the Roberto branch.
Stuff like generate things we need for the Ubuntu live installer.
Those are things we need but maybe you don't need/want, so we are
doing for us right now. We have no much time now to have the system
working fine, but after the rush we like to doing better.
>> Well, the last feature we needed and Roberto has implemented (trying to
>> doing similar to the rest of the code and being as less intrusive we
>> could) was a gfxboot support.
>
> first of all, gfxboot is a bad idea since it makes you being locked into
> an upstream-wise unsupported version of syslinux, and gfx patches tend
> to be updated not very regularly, means you also don't profit from new
> syslinux versions immediately.
>
> i think that the debian approach by using vesamenu is saner. also note,
> that there are experimental patches arround (which should be iirc in a
> branch in upstreams syslinux.git by now) that reimplemented gfxboot as a
> com32 module for syslinux, and they will eventually be merged into
> mainline syslinux, which is nice.
>
> and finally, gfxboot is not available in debian :)
>
> but you probably already knew all of this anyway.
Actually, I didn't.
I understand your point of view. We did it because we need that
graphical menu for our distro and the syslinux from Ubuntu support
gfxboot.
I thought the Debian's as well and if not, syslinux goes to vesamenu
anyways. Also if you are using Debian, you probably know Debian does't
support yet gfxboot, so you won't activate the option for gfxboot.
Our "support" is not actually "gfxboot support for syslinux", but
"gfxboot theme support for live-helper". I mean, this patch add the
files for the gfxboot theme and the options to the cfg file for
booting graphically if syslinux and the graphic card support it.
Ubuntu has patch from Colin Watson to add the gfxboot support to the
Ubuntu syslinux and is being used since a couple of versions. I don't
know if those are the patches you were talking.
Anyway, we've just needed that feature (to be able of use a gfxboot
theme on our live systems) and we thought Debian syslinux had also
support for that so could be nice to do it for both. We don't want
force or invite to no one to use non-uptream patches to use that
feature or so.
>> We hope you like it and if you have any though, advise or comment we
>> love to read it :-)
>
> generally, i love to apply patches and to add stuff that make other
> peoples live easier. however, i've three basic things why i'm reluctant
> here. please read on.
>
> first, i think that gfxboot is just an option to syslinux, so it should,
> if at all, being incorporated into lh_binary_syslinux directly and
> handled be there. imho, it should be handled the similar as the vesamenu
> and therefore doesn't warrant a new helper (we have already a lot of
> them :).
This is my bad. I told Roberto to do it this way to avoid touch too
much code, to make the patch the less intrusive as possible. His idea
was more in your line.
We didn't know the better way to add things like that so I made
suppositions and I guessed this could be a non very intrusive way. But
you're right, it's more a syslinux option than another thing.
> second, given that the /current/ gfxboot is more like a temporary hack
> until gfxboot.c32 was mainlined, i think that the legacy gfxboot
> handling would be better suited as a local-hook in binary stage
> (lh_binary_local-hooks). once it's available as com32 module, it can be
> very efficiently handled as alternative to vesamenu.c32.
yep, probably it's a good idea, make it by hooks. But I still don't
see it like a temporary hack because this does't give any gfxboot
support to syslinux, just add the theme and if the syslinux support it
(as the Ubuntu one) it be used.
Also it could be a part of the Ubuntu mode of live-helper and only be
activated when the system selected be Ubuntu.
What do you think?
> third, by assigning LH_* variables to it, it becomes part of the
> interface that frontends based on live-helper should implement. assumed
> that gfxboot gets mainlined in weeks or months, this may be tedious to
> add it and then revoke or change it again. also, i feel bad about having
> 'official' options in live-helper which are not actually
> usable/resolvable at all within debian itself.
Well, the LH_* variables don't need to be different. I mean, as far i
know, you don't have to do nothing but add some options to the
syslinux/isolinux.cfg file and the theme to the directory for making
the syslinux boot the graphical mode but you don't need to do nothing
to syslinux. I don't see which different options or variables are
possible there.
I'm not syslinux hacker so I might be missing something.
> what do you think? other opinions are of course welcome.
>
>> I attach the patch file.
>
> some comments regarding coding style:
>
> +lh_binary_enable_gfxboot ${*}
>
> helpers names are generally short, so lh_binary_gfxboot would be better
> (if it would be in an own helper).
Annotated :-)
> +# Copyright (C) 2009 Roberto C. Morano <rcmorano@emergya.es>
>
> for the moment, i'd like to keep copyright on all helpers consistently
> assigned to me. is this a problem for you?
No really, it was just the usual way to work.
> +if [ ! -z ${LH_GFXBOOT_THEME} ]
>
> use the short test '-n' here (also on all places below this).
Yes, you're right. We'll change it. Thanks :-)
> +GFXBOOT_TGZ="$GFXBOOT_THEME/bootlogo.tar.gz"
>
> do use curly brackets for variables (also on all places below this).
Again, you're right. We'll change it as well.
> + Echo_error "gfxboot-theme-$GFXBOOT_THEME not installed"
> + exit 1
>
> i think, the build should not fail here, but just give a warning that
> although gfxboot was specified, it was not found and therefore ignored.
> it would be consistend on how win32-loader is handled and imho sucks
> less for the user :))
>
> + cp -a $GFXBOOT_LANG binary/isolinux/
>
> why 'cp -a' here, isn't 'cp' enough?
That's because $GFXBOOT_LANG is a directory. Cold be just "cp -r",
but must be, at least, a recursive option.
[...]
Thanks again for your time and your comments. Now we know more about
how to touch the code and we can fit a bit more into it.
We'll probably be around here in the list and we'll ask how to write
better other features :-)
Greetings
--
Juanje
Reply to: