diff mbox

pkg-infra: limit -reconfigure and -rebuild actions

Message ID 1342791572-18657-1-git-send-email-rbraun@sceen.net
State Superseded
Headers show

Commit Message

Richard Braun July 20, 2012, 1:39 p.m. UTC
The -reconfigure and -rebuild per package targets unconditionally
recreate the root filesystem image by depending on the all target.
Restrict their actions to their package instead.

Signed-off-by: Richard Braun <rbraun@sceen.net>
---
 package/pkg-generic.mk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Émeric Vigier July 20, 2012, 3:34 p.m. UTC | #1
Thanks for coming up with a patch following yesterday discussion. Got some comments though:

----- Mail original -----
> The -reconfigure and -rebuild per package targets unconditionally
> recreate the root filesystem image by depending on the all target.
> Restrict their actions to their package instead.
> 
> Signed-off-by: Richard Braun <rbraun@sceen.net>
> ---
>  package/pkg-generic.mk |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index c01440e..f5b05a3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -389,12 +389,12 @@ endif
>  			rm -f $$($(2)_TARGET_INSTALL_IMAGES)
>  			rm -f $$($(2)_TARGET_INSTALL_HOST)
>  
> -$(1)-rebuild:		$(1)-clean-for-rebuild all
> +$(1)-rebuild:		$(1)-clean-for-rebuild $(1)
>  
>  $(1)-clean-for-reconfigure: $(1)-clean-for-rebuild
>  			rm -f $$($(2)_TARGET_CONFIGURE)
>  
> -$(1)-reconfigure:	$(1)-clean-for-reconfigure all
> +$(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)-configure

Your patch makes -reconfigure clean-for-rebuild and then (just) -configure, without rebuilding.
That's ok to me, but you have to choose between:

1. updating docs/manual/rebuilding-packages.txt accordingly, especially:
"For convenience, most packages support the special make targets
<package>-reconfigure and <package>-rebuild to repeat the configure
and build steps"

2. or keep -reconfigure rebuilding the entire package, i.e.:
-+$(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)-configure
++$(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)

>  
>  # define the PKG variable for all targets, containing the
>  # uppercase package variable prefix
> --
> 1.7.2.5
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Richard Braun July 20, 2012, 3:58 p.m. UTC | #2
On Fri, Jul 20, 2012 at 11:34:44AM -0400, Émeric Vigier wrote:
> Your patch makes -reconfigure clean-for-rebuild and then (just) -configure, without rebuilding.
> That's ok to me, but you have to choose between:
> 
> 1. updating docs/manual/rebuilding-packages.txt accordingly, especially:
> "For convenience, most packages support the special make targets
> <package>-reconfigure and <package>-rebuild to repeat the configure
> and build steps"

I may misunderstand what you mean, because, when reading this part, I
understand that "-reconfigure and -rebuild" "repeat the configure and
build steps". It may lack a formal "respectively", but other than that,
it's exactly what my patch does by calling only -configure.
Thomas Petazzoni July 20, 2012, 8:27 p.m. UTC | #3
Hello Richard,

Le Fri, 20 Jul 2012 15:39:32 +0200,
Richard Braun <rbraun@sceen.net> a écrit :

> The -reconfigure and -rebuild per package targets unconditionally
> recreate the root filesystem image by depending on the all target.
> Restrict their actions to their package instead.
> 
> Signed-off-by: Richard Braun <rbraun@sceen.net>

Well, the use case for those targets was the following: I am making
modifications to the source code of a package (especially using the
OVERRIDE_SRCDIR mechanism) and I want to easily restart the build of
that package and recreate the filesystem image so that I can test the
new version of my package on the target. That's the reason why the
target restarts the build *and* recreates the target filesystem.

For the -reconfigure, the idea is the same: I made some changes that
affect the configure script, or changed configuration options, and I
want to restart the configure+build+install of my package and recreate
the target filesystem.

That's the original idea of those targets. That said, I am personally
open to making changes to their semantic, but I'd prefer to see what
the opinion of other people in the community.

Best regards,

Thomas
Émeric Vigier July 20, 2012, 9:55 p.m. UTC | #4
----- Mail original -----
> On Fri, Jul 20, 2012 at 11:34:44AM -0400, Émeric Vigier wrote:
> > Your patch makes -reconfigure clean-for-rebuild and then (just)
> > -configure, without rebuilding.
> > That's ok to me, but you have to choose between:
> > 
> > 1. updating docs/manual/rebuilding-packages.txt accordingly,
> > especially:
> > "For convenience, most packages support the special make targets
> > <package>-reconfigure and <package>-rebuild to repeat the configure
> > and build steps"
> 
> I may misunderstand what you mean, because, when reading this part, I
> understand that "-reconfigure and -rebuild" "repeat the configure and
> build steps". It may lack a formal "respectively", but other than
> that,
> it's exactly what my patch does by calling only -configure.

Indeed respectively is missing.

> 
> --
> Richard Braun
>
Richard Braun July 20, 2012, 11:31 p.m. UTC | #5
On Fri, Jul 20, 2012 at 10:27:22PM +0200, Thomas Petazzoni wrote:
> Well, the use case for those targets was the following: I am making
> modifications to the source code of a package (especially using the
> OVERRIDE_SRCDIR mechanism) and I want to easily restart the build of
> that package and recreate the filesystem image so that I can test the
> new version of my package on the target. That's the reason why the
> target restarts the build *and* recreates the target filesystem.

To clarify my point of view: the intended behaviour is obvious, and
I assumed the original author knew what he wanted when these targets
were added. But my use cases are rather the following: I am making
modifications to the source code of a package, and I want to easily use
the environment provided by buildroot (the toolchain, the staging
directory and the target optimizations, maybe others). If it fails, it
will stop in either case. But if it succeeds, I want to easily check the
output and make sure the build process produced the expected results
(I mostly check compiler warnings, sometimes the generated code). If I
am confident in the results, I just add && make (which is short and
simple enough) to get the current behaviour.

I usually use buildroot with two flavors of configuration: a production
one, and a developer one (with debugging symbols, target debugging tools
such as valgrind, sometimes -O0 executables). This can result in huge
images, so getting to that step while the build of the package I'm
working on didn't reach a state I can confidently validate doesn't make
sense. And when it does (and even if it doesn't), simply adding
"&& make" does the job.

I'd also be glad to read from others, though, and hope you give much
attention to the code you're working on too :-).
Thomas Petazzoni July 21, 2012, 12:54 p.m. UTC | #6
Le Sat, 21 Jul 2012 01:31:36 +0200,
Richard Braun <rbraun@sceen.net> a écrit :

> To clarify my point of view: the intended behaviour is obvious, and
> I assumed the original author knew what he wanted when these targets
> were added.

Actually, the original author was me :-)

See 4ed4e5016b741341059ed826416dad3291df0b2c.

> But my use cases are rather the following: I am making
> modifications to the source code of a package, and I want to easily use
> the environment provided by buildroot (the toolchain, the staging
> directory and the target optimizations, maybe others). If it fails, it
> will stop in either case. But if it succeeds, I want to easily check the
> output and make sure the build process produced the expected results
> (I mostly check compiler warnings, sometimes the generated code). If I
> am confident in the results, I just add && make (which is short and
> simple enough) to get the current behaviour.
> 
> I usually use buildroot with two flavors of configuration: a production
> one, and a developer one (with debugging symbols, target debugging tools
> such as valgrind, sometimes -O0 executables). This can result in huge
> images, so getting to that step while the build of the package I'm
> working on didn't reach a state I can confidently validate doesn't make
> sense. And when it does (and even if it doesn't), simply adding
> "&& make" does the job.

It does make some sense, but I'd like to see the opinion of others.

However, I don't agree with the change you did on -reconfigure that
would only to the configure step. If -rebuild does build+install, then
-reconfigure should do configure+build+install.

Thomas
Richard Braun July 21, 2012, 3:17 p.m. UTC | #7
On Sat, Jul 21, 2012 at 02:54:11PM +0200, Thomas Petazzoni wrote:
> However, I don't agree with the change you did on -reconfigure that
> would only to the configure step. If -rebuild does build+install, then
> -reconfigure should do configure+build+install.

Right, after a bit more thinking, I agree.
Samuel Martin July 21, 2012, 4:14 p.m. UTC | #8
Hi all,

Here, I'll try to sum up what we talked few days ago on the IRC
channel, plus give my opinion about this.

To be honest, the first time i tried these -reconfigure and -rebuild
targets, I was surprised they didn't behave as I would expect from
targets named like that, rebuilding not only the package but the
images too. So, I keep doing things by hands... though I understand
why things were implemented like this.

IMO, for anyone who wants to re-{configure,build} a package, there are
several use cases (some of these have already been mentioned in this
thread):
1) integrate a (new) package in BR;
2) experimenting, on the target, the result of modifications in the
source code of a package;
3) add a new package in an image.

IMO, goals 3) is the combination of the first 2 ones, though it's
often the first thing coming in mind.

Because of what BR is, people working with it may follow one or
another of these goals:
- people working on targets and what the target will do at the end
mostly aims goal 2) or 3) (or both!);
- people working on BR, considering it as a distribution (working on
the package integration/upgrade, the infrastructure, etc) focus on 1).

So, depending on the end goal, the expected behavior of the make
targets '-reconfigure' and '-rebuild' may differ:
- for the goal 1) followers: these targets should only rebuild the
given package;
- for the goals 2) and/or 3) followers: these targets should, not only
rebuild the given package, but also the whole regenerate the whole
image.

I'd like 4 targets in BR:
- "-rebuild-single": re-building and re-installing only the given package;
- "-reconfigure-single": re-configuring the given package, then
running 'make <pkg>-rebuild-single';
- "-rebuild-all": running: 'make <pkg>-rebuild-single all';
- "-reconfigure-all": running: 'make <pkg>-reconfigure-single all';

The name of these targets may differ, but the semantic is here.


2012/7/21 Richard Braun <rbraun@sceen.net>:
> On Sat, Jul 21, 2012 at 02:54:11PM +0200, Thomas Petazzoni wrote:
>> However, I don't agree with the change you did on -reconfigure that
>> would only to the configure step. If -rebuild does build+install, then
>> -reconfigure should do configure+build+install.
>
> Right, after a bit more thinking, I agree.
Of course, I second.


Cheers,
Alex Bradbury July 22, 2012, 6:27 p.m. UTC | #9
On 21 July 2012 17:14, Samuel Martin <s.martin49@gmail.com> wrote:
> Hi all,
>
> Here, I'll try to sum up what we talked few days ago on the IRC
> channel, plus give my opinion about this.
>
> To be honest, the first time i tried these -reconfigure and -rebuild
> targets, I was surprised they didn't behave as I would expect from
> targets named like that, rebuilding not only the package but the
> images too. So, I keep doing things by hands... though I understand
> why things were implemented like this.

I'm a newcomer to buildroot and was surprised by the effect of
-reconfigure and -rebuild. I'd read about them in the slides from one
of Tom's talks, and given their description I had assumed they would
rebuild only the package. Reading the responses here, I can see why it
is useful to have a target that rebuilds a single package and also
makes the image (though really, it seems not that hard for the user to
do another make command to build the image in that case). I would love
to see a build target with the proposed semantics.

Alex
Stephan Hoffmann July 22, 2012, 7:20 p.m. UTC | #10
Am 20.07.2012 22:27, schrieb Thomas Petazzoni:
> That's the original idea of those targets. That said, I am personally
> open to making changes to their semantic, but I'd prefer to see what
> the opinion of other people in the community.
>
> Best regards,
>
> Thomas
Hello all,

for my point of view the targets do what I expect. I would opt for
keeping the targets as they are and adding other targets that just
rebuild or reconfiguere a single package.

Kind regards

Stephan
Arnout Vandecappelle July 24, 2012, 11:07 p.m. UTC | #11
Funny how much discussion can rise from a two-line patch :-)

On 07/21/12 18:14, Samuel Martin wrote:
> Hi all,
>
> Here, I'll try to sum up what we talked few days ago on the IRC
> channel, plus give my opinion about this.
>
> To be honest, the first time i tried these -reconfigure and -rebuild
> targets, I was surprised they didn't behave as I would expect from
> targets named like that, rebuilding not only the package but the
> images too. So, I keep doing things by hands... though I understand
> why things were implemented like this.

  Me too.  I tend to run command lines like:

make foo-clean-for-rebuild foo && scp target/usr/bin/foo mytarget:/usr/bin

so I would much appreciate an abbreviated 'make foo-rebuild'.


> IMO, for anyone who wants to re-{configure,build} a package, there are
> several use cases (some of these have already been mentioned in this
> thread):
> 1) integrate a (new) package in BR;
> 2) experimenting, on the target, the result of modifications in the
> source code of a package;
> 3) add a new package in an image.
>
> IMO, goals 3) is the combination of the first 2 ones, though it's
> often the first thing coming in mind.

  I think for 1) you'd use foo-reconfigure, for 2) foo-rebuild, and
for 3) you can actually just run make (after a menuconfig) of make foo.


> Because of what BR is, people working with it may follow one or
> another of these goals:
> - people working on targets and what the target will do at the end
> mostly aims goal 2) or 3) (or both!);
> - people working on BR, considering it as a distribution (working on
> the package integration/upgrade, the infrastructure, etc) focus on 1).
>
> So, depending on the end goal, the expected behavior of the make
> targets '-reconfigure' and '-rebuild' may differ:
> - for the goal 1) followers: these targets should only rebuild the
> given package;
> - for the goals 2) and/or 3) followers: these targets should, not only
> rebuild the given package, but also the whole regenerate the whole
> image.

  I disagree for goal 2).  I think most people (me at least) either use
nfsroot or scp during development, and don't go through the process
of flashing a full rootfs for every experiment (at the rate that I
produce bugs, the wear on MFC NAND would be horrible :-).


> I'd like 4 targets in BR:
> - "-rebuild-single": re-building and re-installing only the given package;
> - "-reconfigure-single": re-configuring the given package, then
> running 'make<pkg>-rebuild-single';
> - "-rebuild-all": running: 'make<pkg>-rebuild-single all';
> - "-reconfigure-all": running: 'make<pkg>-reconfigure-single all';
>
> The name of these targets may differ, but the semantic is here.

  You almost indicate yourself that the two latter targets add little
value...  'make foo-rebuild all' is exactly the same amount of typing
as 'make foo-rebuild-all'...  I'd be disappointed to need the extra
typing of a -single.  And I'm also not so happy about adding more
per-package targets, as these will inevitable make tab completion
even slower.

  Bottom line: I'm all in favour of Richard's original patch
(except that foo-reconfigure should also reinstall foo, which
everybody seems to agree with).

  Regards,
  Arnout

>
>
> 2012/7/21 Richard Braun<rbraun@sceen.net>:
>> On Sat, Jul 21, 2012 at 02:54:11PM +0200, Thomas Petazzoni wrote:
>>> However, I don't agree with the change you did on -reconfigure that
>>> would only to the configure step. If -rebuild does build+install, then
>>> -reconfigure should do configure+build+install.
>>
>> Right, after a bit more thinking, I agree.
> Of course, I second.
>
>
> Cheers,
>
Alex Bradbury Aug. 2, 2012, 1:28 p.m. UTC | #12
On 25 July 2012 00:07, Arnout Vandecappelle <arnout@mind.be> wrote:
>  Bottom line: I'm all in favour of Richard's original patch
> (except that foo-reconfigure should also reinstall foo, which
> everybody seems to agree with).

So has this patch missed 2012.08, or might it still be included?

Alex
Thomas Petazzoni Aug. 2, 2012, 1:36 p.m. UTC | #13
Le Thu, 2 Aug 2012 14:28:37 +0100,
Alex Bradbury <asb@asbradbury.org> a écrit :

> On 25 July 2012 00:07, Arnout Vandecappelle <arnout@mind.be> wrote:
> >  Bottom line: I'm all in favour of Richard's original patch
> > (except that foo-reconfigure should also reinstall foo, which
> > everybody seems to agree with).
> 
> So has this patch missed 2012.08, or might it still be included?

It has missed 2012.08, I haven't the time to get to it, and now that
-rc1 has been released, we don't accept such changes.

There was a fairly long backlog of patches to handle, and I only had
from July 13th to July 31th to try to catch up. I tried to merge things
fairly aggressively during that time, but didn't manage to merge
everything.

However, while we are in the process of preparing 2012.08, I will open
a 'next' branch in which I will accumulate the patches that will be
merged as soon as the 2012.11 development cycle open, at the end of
August/early September. Therefore, I am hoping to reduce even further
the backlog by merging stuff into the 'next' branch.

Best regards,

Thomas
Alex Bradbury Sept. 21, 2012, 4:57 p.m. UTC | #14
On 02/08/2012, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Le Thu, 2 Aug 2012 14:28:37 +0100,
> Alex Bradbury <asb@asbradbury.org> a écrit :
>> So has this patch missed 2012.08, or might it still be included?
>
> It has missed 2012.08, I haven't the time to get to it, and now that
> -rc1 has been released, we don't accept such changes.

So we're in to 2012.11 now, and unless I missed it no patch to address
the concerns raised in the this thread has been committed. It looks
like there was quite a lot of general agreement about this patch (see
Samuel Martin's summary and Arnout's later followup). Any thoughts
Peter?

Regards,

Alex
Arnout Vandecappelle Dec. 14, 2012, 1:49 p.m. UTC | #15
On 22/07/12 20:27, Alex Bradbury wrote:
> On 21 July 2012 17:14, Samuel Martin<s.martin49@gmail.com>  wrote:
>> Hi all,
>>
>> Here, I'll try to sum up what we talked few days ago on the IRC
>> channel, plus give my opinion about this.
>>
>> To be honest, the first time i tried these -reconfigure and -rebuild
>> targets, I was surprised they didn't behave as I would expect from
>> targets named like that, rebuilding not only the package but the
>> images too. So, I keep doing things by hands... though I understand
>> why things were implemented like this.
>
> I'm a newcomer to buildroot and was surprised by the effect of
> -reconfigure and -rebuild. I'd read about them in the slides from one
> of Tom's talks, and given their description I had assumed they would
> rebuild only the package. Reading the responses here, I can see why it
> is useful to have a target that rebuilds a single package and also
> makes the image (though really, it seems not that hard for the user to
> do another make command to build the image in that case). I would love
> to see a build target with the proposed semantics.

  A colleague of mine (again) made this remark to me today... So, will we
accept this change or not?

  Clearly it changes the behaviour, but I don't expect people will use
-rebuild or -reconfigure in scripts so it shouldn't hurt.


  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index c01440e..f5b05a3 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -389,12 +389,12 @@  endif
 			rm -f $$($(2)_TARGET_INSTALL_IMAGES)
 			rm -f $$($(2)_TARGET_INSTALL_HOST)
 
-$(1)-rebuild:		$(1)-clean-for-rebuild all
+$(1)-rebuild:		$(1)-clean-for-rebuild $(1)
 
 $(1)-clean-for-reconfigure: $(1)-clean-for-rebuild
 			rm -f $$($(2)_TARGET_CONFIGURE)
 
-$(1)-reconfigure:	$(1)-clean-for-reconfigure all
+$(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)-configure
 
 # define the PKG variable for all targets, containing the
 # uppercase package variable prefix