Patchwork [2/2] cml1.bbclass: Add fragmentconfig task to cml1

login
register
mail settings
Submitter João Henrique Freitas
Date Feb. 13, 2014, 12:20 a.m.
Message ID <1392250819-10123-3-git-send-email-joaohf@gmail.com>
Download mbox | patch
Permalink /patch/66665/
State New
Headers show

Comments

João Henrique Freitas - Feb. 13, 2014, 12:20 a.m.
fragmentconfig() is a new task that makes a diff between the
old and new config files and writes to the fragment.cfg result file.
menuconfig() always copy the original config file, so the user
doesn't need to copy it.

Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
---
 meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
Darren Hart - Feb. 13, 2014, 12:45 a.m.
On 2/12/14, 16:20, "João Henrique Ferreira de Freitas" <joaohf@gmail.com>
wrote:

>fragmentconfig() is a new task that makes a diff between the
>old and new config files and writes to the fragment.cfg result file.
>menuconfig() always copy the original config file, so the user
>doesn't need to copy it.

Hi João,

Generally looks pretty good. Some comments below, mostly about usability
and robustness.

I do have reservations about the task name, partly because it's long and
hard to type.... I'm not coming up with something I like a lot better
though.... Maybe....

$ bitbake linux-yocto -c diffcfg

Or

$ bitbake linux-yocto -c diffconfig

I like the latter because it is consistent in naming, <action>config, with
menuconfig.

>
>Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
>---
> meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
>index e292ecb..2053361 100644
>--- a/meta/classes/cml1.bbclass
>+++ b/meta/classes/cml1.bbclass
>@@ -16,8 +16,12 @@ HOST_LOADLIBES = "-lncurses"
> TERMINFO = "${STAGING_DATADIR_NATIVE}/terminfo"
> 
> python do_menuconfig() {
>+    import os
>+    import shutil
>+
>     try:
>         mtime = os.path.getmtime(".config")
>+        shutil.copy(".config", ".config.orig")
>     except OSError:
>         mtime = 0
> 
>@@ -38,3 +42,34 @@ do_menuconfig[depends] +=
>"ncurses-native:do_populate_sysroot"
> do_menuconfig[nostamp] = "1"
> addtask menuconfig after do_configure
> 
>+python do_fragmentconfig() {
>+    import shutil
>+    import subprocess
>+
>+    workdir = d.getVar('WORKDIR', True)
>+    fragment = workdir + '/fragment.cfg'
>+    configorig = '.config.orig'
>+    config = '.config'
>+
>+    try:
>+        md5newconfig = bb.utils.md5_file(configorig)
>+        md5config = bb.utils.md5_file(config)
>+        isdiff = md5newconfig != md5config
>+    except OSError:

We should be doing something to indicate failure here.

>+        isdiff = 0
>+
>+    if isdiff:
>+        bb.note("Dumping config fragment into: '%s'." % fragment)
>+        bb.note("new '%s' old '%s'" % (md5newconfig,md5config))

Nit, add space after ","

>+
>+        statement = 'diff -Nurp ' + configorig + ' ' + config + '| sed
>-n "s/^\+//p" >' + fragment
>+        subprocess.call(statement, shell=True)
>+
>+        shutil.copy(configorig, config)

We should be printing something to make it easy for the user to find the
fragment.

>+    else:
>+        if os.path.exists(fragment):
>+            os.unlink(fragment)
>+}
>+
>+do_fragmentconfig[nostamp] = "1"
>+addtask fragmentconfig
>-- 
>1.8.3.2
>
>
Richard Purdie - Feb. 13, 2014, 11:33 a.m.
On Wed, 2014-02-12 at 22:20 -0200, João Henrique Ferreira de Freitas
wrote:
> fragmentconfig() is a new task that makes a diff between the
> old and new config files and writes to the fragment.cfg result file.
> menuconfig() always copy the original config file, so the user
> doesn't need to copy it.
> 
> Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
> ---
>  meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
> index e292ecb..2053361 100644
> --- a/meta/classes/cml1.bbclass
> +++ b/meta/classes/cml1.bbclass
> @@ -16,8 +16,12 @@ HOST_LOADLIBES = "-lncurses"
>  TERMINFO = "${STAGING_DATADIR_NATIVE}/terminfo"
>  
>  python do_menuconfig() {
> +    import os

Don't import os please, this is always present so we don't need to.

> +    import shutil
> +
>      try:
>          mtime = os.path.getmtime(".config")
> +        shutil.copy(".config", ".config.orig")
>      except OSError:
>          mtime = 0
>  
> @@ -38,3 +42,34 @@ do_menuconfig[depends] += "ncurses-native:do_populate_sysroot"
>  do_menuconfig[nostamp] = "1"
>  addtask menuconfig after do_configure
>  
> +python do_fragmentconfig() {
> +    import shutil
> +    import subprocess
> +
> +    workdir = d.getVar('WORKDIR', True)
> +    fragment = workdir + '/fragment.cfg'
> +    configorig = '.config.orig'
> +    config = '.config'
> +
> +    try:
> +        md5newconfig = bb.utils.md5_file(configorig)
> +        md5config = bb.utils.md5_file(config)
> +        isdiff = md5newconfig != md5config
> +    except OSError:
> +        isdiff = 0
> +
> +    if isdiff:
> +        bb.note("Dumping config fragment into: '%s'." % fragment)
> +        bb.note("new '%s' old '%s'" % (md5newconfig,md5config))
> +
> +        statement = 'diff -Nurp ' + configorig + ' ' + config + '| sed -n "s/^\+//p" >' + fragment
> +        subprocess.call(statement, shell=True)
> +
> +        shutil.copy(configorig, config)
> +    else:
> +        if os.path.exists(fragment):
> +            os.unlink(fragment)
> +}
> +
> +do_fragmentconfig[nostamp] = "1"
> +addtask fragmentconfig
Darren Hart - Feb. 13, 2014, 7 p.m.
On 2/13/14, 3:33, "Richard Purdie" <richard.purdie@linuxfoundation.org>
wrote:

>On Wed, 2014-02-12 at 22:20 -0200, João Henrique Ferreira de Freitas
>wrote:
>> fragmentconfig() is a new task that makes a diff between the
>> old and new config files and writes to the fragment.cfg result file.
>> menuconfig() always copy the original config file, so the user
>> doesn't need to copy it.
>> 
>> Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
>> ---
>>  meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>> 
>> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
>> index e292ecb..2053361 100644
>> --- a/meta/classes/cml1.bbclass
>> +++ b/meta/classes/cml1.bbclass

One more thought on this. As this is currently linux-yocto specific, does
it belong in cml1.bbclass? It may make sense in busybox as well, so maybe
this is the right place.

RP, do you have a preference?
Richard Purdie - Feb. 14, 2014, 3:46 p.m.
On Thu, 2014-02-13 at 11:00 -0800, Darren Hart wrote:
> On 2/13/14, 3:33, "Richard Purdie" <richard.purdie@linuxfoundation.org>
> wrote:
> 
> >On Wed, 2014-02-12 at 22:20 -0200, João Henrique Ferreira de Freitas
> >wrote:
> >> fragmentconfig() is a new task that makes a diff between the
> >> old and new config files and writes to the fragment.cfg result file.
> >> menuconfig() always copy the original config file, so the user
> >> doesn't need to copy it.
> >> 
> >> Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
> >> ---
> >>  meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
> >>  1 file changed, 35 insertions(+)
> >> 
> >> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
> >> index e292ecb..2053361 100644
> >> --- a/meta/classes/cml1.bbclass
> >> +++ b/meta/classes/cml1.bbclass
> 
> One more thought on this. As this is currently linux-yocto specific, does
> it belong in cml1.bbclass? It may make sense in busybox as well, so maybe
> this is the right place.
> 
> RP, do you have a preference?

Its starting to head in a direction we could probably use a
kern-tools.bbclass to keep the code in a common place.

Cheers,

Richard
Darren Hart - Feb. 14, 2014, 4:15 p.m.
On 2/14/14, 7:46, "Richard Purdie" <richard.purdie@linuxfoundation.org>
wrote:

>On Thu, 2014-02-13 at 11:00 -0800, Darren Hart wrote:
>> On 2/13/14, 3:33, "Richard Purdie" <richard.purdie@linuxfoundation.org>
>> wrote:
>> 
>> >On Wed, 2014-02-12 at 22:20 -0200, João Henrique Ferreira de Freitas
>> >wrote:
>> >> fragmentconfig() is a new task that makes a diff between the
>> >> old and new config files and writes to the fragment.cfg result file.
>> >> menuconfig() always copy the original config file, so the user
>> >> doesn't need to copy it.
>> >> 
>> >> Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
>> >> ---
>> >>  meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
>> >>  1 file changed, 35 insertions(+)
>> >> 
>> >> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
>> >> index e292ecb..2053361 100644
>> >> --- a/meta/classes/cml1.bbclass
>> >> +++ b/meta/classes/cml1.bbclass
>> 
>> One more thought on this. As this is currently linux-yocto specific,
>>does
>> it belong in cml1.bbclass? It may make sense in busybox as well, so
>>maybe
>> this is the right place.
>> 
>> RP, do you have a preference?
>
>Its starting to head in a direction we could probably use a
>kern-tools.bbclass to keep the code in a common place.

I wonder if something like kconfig.bbclass, rather than tying it to the
linux-yocto tooling (as it doesn't use any of the tooling to create the
fragment). Busybox could then use this, for example (not that it couldn't
include kern-tools.bbclass, it just seems a bit less natural to do so).
Richard Purdie - Feb. 14, 2014, 4:44 p.m.
On Fri, 2014-02-14 at 08:15 -0800, Darren Hart wrote:
> On 2/14/14, 7:46, "Richard Purdie" <richard.purdie@linuxfoundation.org>
> wrote:
> 
> >On Thu, 2014-02-13 at 11:00 -0800, Darren Hart wrote:
> >> On 2/13/14, 3:33, "Richard Purdie" <richard.purdie@linuxfoundation.org>
> >> wrote:
> >> 
> >> >On Wed, 2014-02-12 at 22:20 -0200, João Henrique Ferreira de Freitas
> >> >wrote:
> >> >> fragmentconfig() is a new task that makes a diff between the
> >> >> old and new config files and writes to the fragment.cfg result file.
> >> >> menuconfig() always copy the original config file, so the user
> >> >> doesn't need to copy it.
> >> >> 
> >> >> Signed-off-by: João Henrique Ferreira de Freitas <joaohf@gmail.com>
> >> >> ---
> >> >>  meta/classes/cml1.bbclass | 35 +++++++++++++++++++++++++++++++++++
> >> >>  1 file changed, 35 insertions(+)
> >> >> 
> >> >> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
> >> >> index e292ecb..2053361 100644
> >> >> --- a/meta/classes/cml1.bbclass
> >> >> +++ b/meta/classes/cml1.bbclass
> >> 
> >> One more thought on this. As this is currently linux-yocto specific,
> >>does
> >> it belong in cml1.bbclass? It may make sense in busybox as well, so
> >>maybe
> >> this is the right place.
> >> 
> >> RP, do you have a preference?
> >
> >Its starting to head in a direction we could probably use a
> >kern-tools.bbclass to keep the code in a common place.
> 
> I wonder if something like kconfig.bbclass, rather than tying it to the
> linux-yocto tooling (as it doesn't use any of the tooling to create the
> fragment). Busybox could then use this, for example (not that it couldn't
> include kern-tools.bbclass, it just seems a bit less natural to do so).

Well, busybox does DEPEND on the kern-tools-native package. cml1.bbclass
is effectively a kconfig.bbclass. I'd be ok with this code there, the
fragment is kern-tools specific at this point but its not a big deal...

Cheers,

Richard
João Henrique Freitas - Feb. 15, 2014, 11:51 p.m.
Hi Darren,

> $ bitbake linux-yocto -c diffconfig
> 
> I like the latter because it is consistent in naming, <action>config,
> with menuconfig.
> 

Ok.

> >+    try:
> >+        md5newconfig = bb.utils.md5_file(configorig)
> >+        md5config = bb.utils.md5_file(config)
> >+        isdiff = md5newconfig != md5config
> >+    except OSError:
> 
> We should be doing something to indicate failure here.

Ok. 

> 
> >+
> >+        statement = 'diff -Nurp ' + configorig + ' ' + config + '|
> >sed -n "s/^\+//p" >' + fragment
> >+        subprocess.call(statement, shell=True)
> >+
> >+        shutil.copy(configorig, config)
> 
> We should be printing something to make it easy for the user to find
> the fragment.

I've suppose that the bb.note did it. But  the messages doesn't appear
on console. Only in the log.do_diffconfig log file.

What is the trick here? Should I use bb.warn() or logger.info() ?

Thanks.

Patch

diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
index e292ecb..2053361 100644
--- a/meta/classes/cml1.bbclass
+++ b/meta/classes/cml1.bbclass
@@ -16,8 +16,12 @@  HOST_LOADLIBES = "-lncurses"
 TERMINFO = "${STAGING_DATADIR_NATIVE}/terminfo"
 
 python do_menuconfig() {
+    import os
+    import shutil
+
     try:
         mtime = os.path.getmtime(".config")
+        shutil.copy(".config", ".config.orig")
     except OSError:
         mtime = 0
 
@@ -38,3 +42,34 @@  do_menuconfig[depends] += "ncurses-native:do_populate_sysroot"
 do_menuconfig[nostamp] = "1"
 addtask menuconfig after do_configure
 
+python do_fragmentconfig() {
+    import shutil
+    import subprocess
+
+    workdir = d.getVar('WORKDIR', True)
+    fragment = workdir + '/fragment.cfg'
+    configorig = '.config.orig'
+    config = '.config'
+
+    try:
+        md5newconfig = bb.utils.md5_file(configorig)
+        md5config = bb.utils.md5_file(config)
+        isdiff = md5newconfig != md5config
+    except OSError:
+        isdiff = 0
+
+    if isdiff:
+        bb.note("Dumping config fragment into: '%s'." % fragment)
+        bb.note("new '%s' old '%s'" % (md5newconfig,md5config))
+
+        statement = 'diff -Nurp ' + configorig + ' ' + config + '| sed -n "s/^\+//p" >' + fragment
+        subprocess.call(statement, shell=True)
+
+        shutil.copy(configorig, config)
+    else:
+        if os.path.exists(fragment):
+            os.unlink(fragment)
+}
+
+do_fragmentconfig[nostamp] = "1"
+addtask fragmentconfig