Hi,
On Mon, Aug 18, 2008, Mitsutaka Amano wrote:
> I checked some patches. First, I show you patch status as currently
> moblin-image-creator.
Thanks!
> - 40_fsets.patch
> We'll merge it.
Glad to read that; just a comment on the various changes in there:
* Ubuntu MID doesn't use flash, even in the private builds, but the USB
client packages are expected: remove flashplugin-nonfree and add
mobile-usb-client-utils + mobile-usb-host-utils in the
CrownBeach-Full-Mobile-Stack-with-Proprietary and
Samsung-Full-Mobile-Stack-with-Proprietary meta-packages
* telepathy dropped from GNOME-Mobile fset: I think telepathy was
dropped because the empathy fork of moblin (mobile-chat) was dropped
in favor of pidgin
* Install only the ubuntu-mobile package in the Ubuntu-Mobile fset: we
don't manage installed packages in Ubuntu MID by matters of fsets;
instead we use the same mechanism as for all Ubuntu flavors which is
"seeds". For instance, Ubuntu, Kubuntu, Xubuntu, Ubuntu Studio etc.
all use seeds to define which packages should be installed. The
ubuntu-mobile meta-package will install all packages in the mobile
seed, so it's enough to install it to get Ubuntu MID installed; it
would be a bad idea to duplicate the information in two places.
* Install marvell-8686-firmware-9 in the
CrownBeach-Full-Mobile-Stack-with-Proprietary fset; I guess this is
to support a proprietary wifi chip which is present in some ODM
hardware
* Install Poulsbo 3D accelerated libs in
CrownBeach-Full-Mobile-Stack-with-Proprietary fset: psb-video
libgl1-mesa-dri-psb xorg-modules-xpsb
> - 60_var-lib-projects.patch
> It's old patch. It doesn't need.
Hmm older MICs would create a var/lib/moblin-image-creator/projects. I
think it might be more elegant to create such a directory at install
time rather than at runtime.
Also, the mkdir is still present in current git MIC:
self.config_path = os.path.join(self.var_dir, 'projects')
if not os.path.isdir(self.config_path):
os.mkdir(self.config_path)
This triggerred Launchpad #232301 in the past; I can't check whether it
still causes this bug as I currently get:
% prefix/bin/image-creator
Error: Must run image creator with sudo or run as root
Example, run image creator: sudo image-creator
> commit fce7eeeb2316f837e147e952063751d7d2ced958
> Author: Prajwal Mohan <[EMAIL PROTECTED]>
> Date: Tue May 27 15:32:14 2008 -0700
>
> * Make file creates projects dir in /usr/share/pdk. Moving it to
> /var/lib/mo
> blin-image-creator
Sure, the location changed a while ago, but note that the creation of
the directory is still done at runtime ATM, instead of build time as in
the proposed patch (see above Launchpad reference).
> - 61_skip-usr-share-projects.patch
> It was already implemented.
Indeed! BTW this code has a note "I would think that after Jun-2008,
we can delete this list"; perhaps time to drop it?
> - 66_mount-boot.patch
> It was already implemented.
>
> commit 90c66f0a732a392a4abf844223762db98bfe2ee8
> Author: Prajwal Mohan <[EMAIL PROTECTED]>
> Date: Mon Jul 14 10:45:39 2008 -0700
>
> Checking in bug fix for 227013. /boot will be mounted rw on boot with disk
> i
> nitramfs script
That's indeed true for common-apt, however I would suspect common-yum's
script suffers from the same issue. The scripts are almost identical
in other respects, but I don't use common-yum so I can't really tell.
I think it would be nice to have a platform independent implementation
of initramfs scripts if possible. Maintaining two flavors of this
infrastructure is probably painful.
Could you perhaps resync the two scripts? I guess both might be able
to use /bin/sh instead of /bin/bash ("checkbashisms" reports no bashism
in them) and the "mount"s probably make sense for yum platforms.
> - 68_clean-default-kopts.patch
> We'll merge partial it.
Ok; what part do you intend to merge / not to merge? The rationale
here is that when you call "update-grub" on Ubuntu systems, you get two
versions of the "kernel" command lines in menu.lst: one without splash
and quiet and one with splash and quiet (the default). Hence, it's not
necessary to add them explicitely (at least not on Ubuntu systems).
> - 69_run-update-grub.patch
> We'll merge partial it.
This patch sets "kopt" in the comments of menu.lst, which is the
mechanism used for update-grub's "automagic" kernel-entries generation
I described earlier. Then it replaces the heavy sed usage with a
simpler one which only sets kopt instead of touching all kernel
entries, and calls update-grub to refresh the kernel entries.
There's a bug in this support: when the image is created, /boot isn't a
separate partition, it's just a directory. So when "update-grub -y"
runs to generate the default menu.lst, it generates kernel entries
relative to the partition holding /boot (which / at this point). This
means the kernel entries are "/boot/something". A checksum of the
initial menu.lst is then recorded. But then on the device /boot is a
separate partition, this is why there is this /boot -> / sed to convert
the kernel entries to "/something" (which will work in the target
device).
The problem here is that the next time you run update-grub, it compares
the checksum which was computed with /boot entries with the new
checksum with / entries and they don't match. This causes an user
prompt which is annoying.
Emmet Hikory worked around this bug by:
a) removing the /boot -> / rewrite (so menu.lst mentions /boot in
kernel entries, which is wrong)
b) adding a /boot -> / symlink in /boot (/boot/boot points to "/" which
is /boot) to allow grub to find the kernels named /boot/something
below /boot
=> this works because the checksums now match again
I would prefer an alternate implementation which would be to have a
separate /boot (fake) when menu.lst is generated during "update-grub
-y", but this is a bit harder. It probably requires mounting a tmpfs
or something similar.
Could you explain which parts of the patch you intend to keep/drop?
What's your preferred way of solving this issue?
> - 70_menu-lst-default.patch
> We'll merge it.
> - 71_install_locale.patch
> We'll merge it.
> - 72_ume-platforms.patch
> We'll merge it.
> - 73_create-kernel-img-conf.patch
> We'll merge it.
Cool; happy to give more details on what each patch does if you need
any.
> - 74_boot-is-really-boot.patch
> We'll merge it.
(That's the reminder of the /boot -> / symlinking I mentionned
earlier.)
Thanks for the review!
--
Loïc Minier
_______________________________________________
dev mailing list
[email protected]
https://www.moblin.org/mailman/listinfo/dev
