Comment 37 for bug 671027

Revision history for this message
Loïc Minier (lool) wrote : Re: [Bug 671027] Re: Add Efika MX Smartbook/Smarttop support

On Mon, Mar 07, 2011, Matt Sealey wrote:
> The reason I want this in there is because right now, we run Ubuntu.

 Or rather you base on Ubuntu; let's call that some kind of mini Ubuntu
 derivative.

> We do not want to wait around
> while Canonical, Linaro or whatever gets their act together and decides
> to perform an ineffectual, lackadaisical rewrite which solves absolutely
> no problems whatsoever.

 I'm taking good note of your thoughts.

> Right now, Ubuntu does not even SHIP an Efika kernel, and the
> code path can never be run on any board you really care about, so I
> don't see what the argument is.

 You're incorrect, Ubuntu ships a Linaro kernel wich supports Efika
 smarttops right now, and I would hope will support smartbooks in the
 future.

> We want this in because it means maintaining Ubuntu support for our
> board gets much easier, and we can support customers better, and then we
> can get on with the needful things which is what you are asking us to
> wait for. This is a workload issue.

 By pushing your delta into Ubuntu, you are making it Ubuntu's problem;
 Ubuntu tries to stay close to Debian, by pushing more Ubuntu-specific
 delta, you're making it even harder for Ubuntu developers to resync
 with Debian.

> To answer your sed question, it is easily answered if you understand
> what sed does. -i means "in place" and takes an argument to rename the
> file as a backup before it edits the file in place. I would think it is
> obvious what it is trying to do, what happens is basically sed takes the
> $tmp.boot.script file and replaces all instances of %KERNELVERSION% in
> it, in place. The original file, before this modification, is backed up
> to $tmp.boot.script.bak by sed. The second call replaces all instances
> of %ROOTPARTITION%, in place on the same file. It will overwrite the
> previous backup, however on failure you only need to see the last backup
> made to see if it did not make any of the pertinent variable changes
> when debugging.

 I was asking about eval specifically; I don't think eval is needed,
 it's generally dangerous and I prefer avoiding it.

> eval is required because without that (i.e. just writing it out as sed
> -i..) the shell will not perform variable substitution inside single
> quotes, which are the usual way of specifying sed arguments. sed will be
> running what is written out verbatim. We obviously want to replace the
> %VARIABLES% with $variables, hence the eval.

 Using eval means that your single quotes gets stripped, and you're not
 in the "usual way of specifying sed arguments" anymore; try:
    eval sed 's/ //'

 You should use double-quotes instead, which is the far more common way
 to do variable substitutions with sed:
    sed "s/foo/$bar/"
 obviously, you have to take care of properly quoting within
 double-quotes, but it's less cumbersome than dealing with the
 two-levels of quoting required for an eval.

> Without eval it would run
> sed -i'.bak' -e's,%KERNELVERSION%,$kvers,g' $tmp.boot.script

 sed -i'.bak' "s,%KERNELVERSION%,$kvers,g" "$tmp.boot.script"

> I am rewriting it (flash-kernel as a concept) now, so that won't be too
> long. I am bored of waiting for Canonical, Linaro and Debian developers
> to fix this.

 Noted.

--
Loïc Minier