Patchwork [0/5,V2] refactor the archiver*.bbclass

login
register
mail settings
Submitter Robert Yang
Date Feb. 24, 2014, 3:56 p.m.
Message ID <cover.1393256563.git.liezhi.yang@windriver.com>
Download mbox
Permalink /patch/67257/
State New
Headers show

Pull-request

git://git.openembedded.org/openembedded-core-contrib rbt/archiver

Comments

Robert Yang - Feb. 24, 2014, 3:56 p.m.
=== V2:
* Fix the warning between different machines which is reported by Martin
* Fix the error when the archiver.bbclass is not inherited but
  ARCHIVER_MODE[type] = "srpm" is set, reported by Ross.
* Fix the archiving for gcc staff which uses the shared source according
  to the recently changes of prefuncs and postfuncs in bitbake, so it only
  works well with bitbake's up to date master branch, if we want to backport it,
  we also need backport the pre/postfuncs related patches.

=== V1:
* The archive*.bbclass didn't work, and there were a few problems, for
  example:
  1) There were a few duplicated code
  2) There was no src_dir.org (or orig), but the diff command still use
     it, and it is not easy to fix this issue if we don't change a lot
     of the code.
  3) It didn't archive the source for the native or gcc
  4) The work flow is not very well
  5) The "subprocess.call('fakeroot cp xxxx'" should be removed
  6) And others ...

* So that we have to refactor it, the benefits are:
  1) Fix the problems and make it work well.
  2) Reduce more than 400 lines in total.
  3) Make it easy to use.

// Robert

The following changes since commit 0e5cfef90ff762b33da6dc301dfc9cb3947c8a02:

  runqemu: enforce right CPU type for qemux86/x86-64 (2014-02-13 17:48:47 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib rbt/archiver
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/archiver

Robert Yang (5):
  classes/archive*.bbclass: remove archive-*-source.bbclass
  archiver.bbclass: refactor it
  package_rpm.bbclass: archive the source to srpm package
  archiver.bbclass: move a few code to copyleft_compliance.bbclass
  local.conf.sample.extended: update for the archiver

 meta/classes/archive-configured-source.bbclass |  65 ---
 meta/classes/archive-original-source.bbclass   |  65 ---
 meta/classes/archive-patched-source.bbclass    |  65 ---
 meta/classes/archiver.bbclass                  | 735 ++++++++-----------------
 meta/classes/copyleft_compliance.bbclass       |  55 +-
 meta/classes/package_rpm.bbclass               |  31 +-
 meta/conf/local.conf.sample.extended           |  72 +--
 7 files changed, 327 insertions(+), 761 deletions(-)
 delete mode 100644 meta/classes/archive-configured-source.bbclass
 delete mode 100644 meta/classes/archive-original-source.bbclass
 delete mode 100644 meta/classes/archive-patched-source.bbclass
Ross Burton - Feb. 25, 2014, 12:40 p.m.
Hi,

One point: enabling the archiver causes the stamps for various steps
to change (fetch, configure, package: wherever it hooks) which means a
full rebuild.  Can the archiver hide itself from the stamps so this
doesn't happen?

This appears to be some of the work required:

+do_configure[vardepsexclude] += "do_deploy_archives"
+do_patch[vardepsexclude] += "do_create_diff_gz"
+do_unpack[vardepsexclude] += "do_ar_unpacked"

But I'm still getting a stamp change in do_package that I haven't
chased down yet.

Ross

On 24 February 2014 15:56, Robert Yang <liezhi.yang@windriver.com> wrote:
> === V2:
> * Fix the warning between different machines which is reported by Martin
> * Fix the error when the archiver.bbclass is not inherited but
>   ARCHIVER_MODE[type] = "srpm" is set, reported by Ross.
> * Fix the archiving for gcc staff which uses the shared source according
>   to the recently changes of prefuncs and postfuncs in bitbake, so it only
>   works well with bitbake's up to date master branch, if we want to backport it,
>   we also need backport the pre/postfuncs related patches.
>
> === V1:
> * The archive*.bbclass didn't work, and there were a few problems, for
>   example:
>   1) There were a few duplicated code
>   2) There was no src_dir.org (or orig), but the diff command still use
>      it, and it is not easy to fix this issue if we don't change a lot
>      of the code.
>   3) It didn't archive the source for the native or gcc
>   4) The work flow is not very well
>   5) The "subprocess.call('fakeroot cp xxxx'" should be removed
>   6) And others ...
>
> * So that we have to refactor it, the benefits are:
>   1) Fix the problems and make it work well.
>   2) Reduce more than 400 lines in total.
>   3) Make it easy to use.
>
> // Robert
>
> The following changes since commit 0e5cfef90ff762b33da6dc301dfc9cb3947c8a02:
>
>   runqemu: enforce right CPU type for qemux86/x86-64 (2014-02-13 17:48:47 +0000)
>
> are available in the git repository at:
>
>   git://git.openembedded.org/openembedded-core-contrib rbt/archiver
>   http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/archiver
>
> Robert Yang (5):
>   classes/archive*.bbclass: remove archive-*-source.bbclass
>   archiver.bbclass: refactor it
>   package_rpm.bbclass: archive the source to srpm package
>   archiver.bbclass: move a few code to copyleft_compliance.bbclass
>   local.conf.sample.extended: update for the archiver
>
>  meta/classes/archive-configured-source.bbclass |  65 ---
>  meta/classes/archive-original-source.bbclass   |  65 ---
>  meta/classes/archive-patched-source.bbclass    |  65 ---
>  meta/classes/archiver.bbclass                  | 735 ++++++++-----------------
>  meta/classes/copyleft_compliance.bbclass       |  55 +-
>  meta/classes/package_rpm.bbclass               |  31 +-
>  meta/conf/local.conf.sample.extended           |  72 +--
>  7 files changed, 327 insertions(+), 761 deletions(-)
>  delete mode 100644 meta/classes/archive-configured-source.bbclass
>  delete mode 100644 meta/classes/archive-original-source.bbclass
>  delete mode 100644 meta/classes/archive-patched-source.bbclass
>
> --
> 1.8.3.1
>
Robert Yang - Feb. 25, 2014, 1:05 p.m.
On 02/25/2014 08:40 PM, Burton, Ross wrote:
> Hi,
>
> One point: enabling the archiver causes the stamps for various steps
> to change (fetch, configure, package: wherever it hooks) which means a
> full rebuild.  Can the archiver hide itself from the stamps so this
> doesn't happen?
>
> This appears to be some of the work required:
>
> +do_configure[vardepsexclude] += "do_deploy_archives"
> +do_patch[vardepsexclude] += "do_create_diff_gz"
> +do_unpack[vardepsexclude] += "do_ar_unpacked"
>

Thanks, sounds reasonable, I will update it.

// Robert


> But I'm still getting a stamp change in do_package that I haven't
> chased down yet.
>
> Ross
>
> On 24 February 2014 15:56, Robert Yang <liezhi.yang@windriver.com> wrote:
>> === V2:
>> * Fix the warning between different machines which is reported by Martin
>> * Fix the error when the archiver.bbclass is not inherited but
>>    ARCHIVER_MODE[type] = "srpm" is set, reported by Ross.
>> * Fix the archiving for gcc staff which uses the shared source according
>>    to the recently changes of prefuncs and postfuncs in bitbake, so it only
>>    works well with bitbake's up to date master branch, if we want to backport it,
>>    we also need backport the pre/postfuncs related patches.
>>
>> === V1:
>> * The archive*.bbclass didn't work, and there were a few problems, for
>>    example:
>>    1) There were a few duplicated code
>>    2) There was no src_dir.org (or orig), but the diff command still use
>>       it, and it is not easy to fix this issue if we don't change a lot
>>       of the code.
>>    3) It didn't archive the source for the native or gcc
>>    4) The work flow is not very well
>>    5) The "subprocess.call('fakeroot cp xxxx'" should be removed
>>    6) And others ...
>>
>> * So that we have to refactor it, the benefits are:
>>    1) Fix the problems and make it work well.
>>    2) Reduce more than 400 lines in total.
>>    3) Make it easy to use.
>>
>> // Robert
>>
>> The following changes since commit 0e5cfef90ff762b33da6dc301dfc9cb3947c8a02:
>>
>>    runqemu: enforce right CPU type for qemux86/x86-64 (2014-02-13 17:48:47 +0000)
>>
>> are available in the git repository at:
>>
>>    git://git.openembedded.org/openembedded-core-contrib rbt/archiver
>>    http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/archiver
>>
>> Robert Yang (5):
>>    classes/archive*.bbclass: remove archive-*-source.bbclass
>>    archiver.bbclass: refactor it
>>    package_rpm.bbclass: archive the source to srpm package
>>    archiver.bbclass: move a few code to copyleft_compliance.bbclass
>>    local.conf.sample.extended: update for the archiver
>>
>>   meta/classes/archive-configured-source.bbclass |  65 ---
>>   meta/classes/archive-original-source.bbclass   |  65 ---
>>   meta/classes/archive-patched-source.bbclass    |  65 ---
>>   meta/classes/archiver.bbclass                  | 735 ++++++++-----------------
>>   meta/classes/copyleft_compliance.bbclass       |  55 +-
>>   meta/classes/package_rpm.bbclass               |  31 +-
>>   meta/conf/local.conf.sample.extended           |  72 +--
>>   7 files changed, 327 insertions(+), 761 deletions(-)
>>   delete mode 100644 meta/classes/archive-configured-source.bbclass
>>   delete mode 100644 meta/classes/archive-original-source.bbclass
>>   delete mode 100644 meta/classes/archive-patched-source.bbclass
>>
>> --
>> 1.8.3.1
>>
>
>
Ross Burton - Feb. 27, 2014, 1:05 p.m.
Hi Robert,

Having a look at this and playing with my own toy archiver makes me
think that this shouldn't be using the postfuncs at all.

The main task, deploy_archives, should depend on the subtasks that it
needs and be simply ordered before do_build.  I see that your code
does this but changes it to be before do_rootfs if the recipe is an
image: I'm not sure I understand why this is as do_rootfs is just a
task that executes before do_build so this shouldn't need
special-casing.

Why did you implement the subtasks as postfuncs instead of normal
tasks?  This has the effect of causing more work to be done if the
archiver needs to run: for example do_ar_unpacked() is a postfunc on
do_unpack so either enabling/disabling the archiver or editing the
archiver class causes the do_unpack hash to change, so everything
afterwards (do_patch, do_configure, do_compile, ...) has to
re-execute.

My toy archiver was implemented as a new task which is after do_fetch
but before do_build.  This means that it executes after fetch has
completed but is outside of the
fetch->patch->configure->compile->install sequence.

https://github.com/rossburton/meta-ross/blob/master/classes/rearchiver.bbclass

(it also has a way of archiving the original sources in a way that
actually archives the original sources)

Ross
Robert Yang - March 4, 2014, 1:27 a.m.
Hi Ross,

Thanks for your reply, please see my comments inline.

On 02/27/2014 09:05 PM, Burton, Ross wrote:
> Hi Robert,
>
> Having a look at this and playing with my own toy archiver makes me
> think that this shouldn't be using the postfuncs at all.
>

Yes, I gree as we had talked.

> The main task, deploy_archives, should depend on the subtasks that it
> needs and be simply ordered before do_build.  I see that your code
> does this but changes it to be before do_rootfs if the recipe is an
> image: I'm not sure I understand why this is as do_rootfs is just a
> task that executes before do_build so this shouldn't need
> special-casing.
>

This part of code is derived from the previous archiver.bbclass, I will
remove it.

> Why did you implement the subtasks as postfuncs instead of normal
> tasks?  This has the effect of causing more work to be done if the

What I had thought was that use the postfuncs will always make sure it
will run, for example, if do_unpack runs, then do_unpack[postfuncs] will
always run, but if we use a new task:

addtask do_ar_unpacked after do_unpack before do_patch

If do_patch doesn't run, the do_ar_unpacked would not run either, as had
talked, we will use new ${S}, so this problem will be gone, I will use
new tasks.

> archiver needs to run: for example do_ar_unpacked() is a postfunc on
> do_unpack so either enabling/disabling the archiver or editing the
> archiver class causes the do_unpack hash to change, so everything
> afterwards (do_patch, do_configure, do_compile, ...) has to
> re-execute.
>
> My toy archiver was implemented as a new task which is after do_fetch
> but before do_build.  This means that it executes after fetch has
> completed but is outside of the
> fetch->patch->configure->compile->install sequence.
>
> https://github.com/rossburton/meta-ross/blob/master/classes/rearchiver.bbclass
>
> (it also has a way of archiving the original sources in a way that
> actually archives the original sources)

Great, thanks, I will send a V3 sooner.

// Robert


>
> Ross
>
>