[Buildroot] missing expat-dirclean target

[Buildroot] missing expat-dirclean target

[Cristian Ionescu-Idbohrn]

Index: expat.mk
===================================================================
--- expat.mk	(revision 19026)
+++ expat.mk	(working copy)
@@ -70,6 +70,9 @@
 	#rm -f $(STAGING_DIR)/usr/bin/xmlwf  $(TARGET_DIR)/usr/bin/xmlwf
 	-$(MAKE) -C $(EXPAT_DIR) clean

+expat-dirclean:
+	rm -rf $(EXPAT_DIR)
+
 #############################################################
 #
 # Toplevel Makefile options


-- 
Cristian

[Buildroot] missing expat-dirclean target

[Bernhard Fischer]

On Sun, Jul 08, 2007 at 02:09:11PM +0200, Cristian Ionescu-Idbohrn wrote:
>Index: expat.mk
>===================================================================
>--- expat.mk	(revision 19026)
>+++ expat.mk	(working copy)
>@@ -70,6 +70,9 @@
> 	#rm -f $(STAGING_DIR)/usr/bin/xmlwf  $(TARGET_DIR)/usr/bin/xmlwf

the clean target must wipe installed stuff from the staging-dir. Please
fix this, too.

> 	-$(MAKE) -C $(EXPAT_DIR) clean
>
>+expat-dirclean:
>+	rm -rf $(EXPAT_DIR)
>+
> #############################################################
> #
> # Toplevel Makefile options

this package uses wrong installation pathes, please fix and test while
you're at it (i don't use it):

                --includedir=/include \
is wrong, whould be /usr/include. Most of these pathes are not needed,
just --prefix=/usr should be enough.

[Buildroot] missing expat-dirclean target

[Cristian Ionescu-Idbohrn]

On Sun, 8 Jul 2007, Bernhard Fischer wrote:

> On Sun, Jul 08, 2007 at 02:09:11PM +0200, Cristian Ionescu-Idbohrn wrote:
> >Index: expat.mk
> >===================================================================
> >--- expat.mk	(revision 19026)
> >+++ expat.mk	(working copy)
> >@@ -70,6 +70,9 @@
> > 	#rm -f $(STAGING_DIR)/usr/bin/xmlwf  $(TARGET_DIR)/usr/bin/xmlwf
>
> the clean target must wipe installed stuff from the staging-dir. Please
> fix this, too.

Looks like stuff is removed both from STAGING_DIR and TARGET_DIR.  Is this
the correct behaviour?

> this package uses wrong installation pathes, please fix and test while
> you're at it (i don't use it):
>
>                 --includedir=/include \
> is wrong, whould be /usr/include. Most of these pathes are not needed,
> just --prefix=/usr should be enough.

Will do.


-- 
Cristian

[Buildroot] missing expat-dirclean target

[Bernhard Fischer]

On Sun, Jul 08, 2007 at 03:07:18PM +0200, Cristian Ionescu-Idbohrn wrote:
>On Sun, 8 Jul 2007, Bernhard Fischer wrote:
>
>> On Sun, Jul 08, 2007 at 02:09:11PM +0200, Cristian Ionescu-Idbohrn wrote:
>> >Index: expat.mk
>> >===================================================================
>> >--- expat.mk	(revision 19026)
>> >+++ expat.mk	(working copy)
>> >@@ -70,6 +70,9 @@
>> > 	#rm -f $(STAGING_DIR)/usr/bin/xmlwf  $(TARGET_DIR)/usr/bin/xmlwf
>>
>> the clean target must wipe installed stuff from the staging-dir. Please
>> fix this, too.
>
>Looks like stuff is removed both from STAGING_DIR and TARGET_DIR.  Is this
>the correct behaviour?

I think removing stuff from both staging_dir and target_dir is the way
to go, yes.

Consider somebody intending to turn off a package. The usual intuitive
way is IMHO to make foo-clean then reconfigure to remove that package.

I'm open to alternative suggestions, of course.

>> this package uses wrong installation pathes, please fix and test while
>> you're at it (i don't use it):
>>
>>                 --includedir=/include \
>> is wrong, whould be /usr/include. Most of these pathes are not needed,
>> just --prefix=/usr should be enough.
>
>Will do.

thanks alot in advance!

[Buildroot] missing expat-dirclean target

[Cristian Ionescu-Idbohrn]

On Sun, 8 Jul 2007, Bernhard Fischer wrote:

> On Sun, Jul 08, 2007 at 03:07:18PM +0200, Cristian Ionescu-Idbohrn wrote:
> >
> > Looks like stuff is removed both from STAGING_DIR and TARGET_DIR.  Is
> > this the correct behaviour?
>
> I think removing stuff from both staging_dir and target_dir is the way
> to go, yes.

Alright.  I still wish every makefile had a target called called
<something>-uninstall to do that.

> Consider somebody intending to turn off a package. The usual intuitive
> way is IMHO to make foo-clean then reconfigure to remove that package.
>
> I'm open to alternative suggestions, of course.

This is how I wish it was done (looks more intuitive to me):

<something>-clean	does the usual 'make clean' stuff
<something>-dirclean	wipes out the build directory
<something>-uninstall	wipes whatever was installed


-- 
Cristian

[Buildroot] missing expat-dirclean target

[Bernhard Fischer]

On Sun, Jul 08, 2007 at 03:27:11PM +0200, Cristian Ionescu-Idbohrn wrote:
>On Sun, 8 Jul 2007, Bernhard Fischer wrote:
>
>> On Sun, Jul 08, 2007 at 03:07:18PM +0200, Cristian Ionescu-Idbohrn wrote:
>> >
>> > Looks like stuff is removed both from STAGING_DIR and TARGET_DIR.  Is
>> > this the correct behaviour?
>>
>> I think removing stuff from both staging_dir and target_dir is the way
>> to go, yes.
>
>Alright.  I still wish every makefile had a target called called
><something>-uninstall to do that.
>
>> Consider somebody intending to turn off a package. The usual intuitive
>> way is IMHO to make foo-clean then reconfigure to remove that package.
>>
>> I'm open to alternative suggestions, of course.
>
>This is how I wish it was done (looks more intuitive to me):
>
><something>-clean	does the usual 'make clean' stuff
><something>-dirclean	wipes out the build directory
><something>-uninstall	wipes whatever was installed

sounds ok to me, i'll let others comment. Care to send a patch if noone
objects withing a couple of days?

cheers,

[Buildroot] missing expat-dirclean target

[Cristian Ionescu-Idbohrn]

On Sun, 8 Jul 2007, Bernhard Fischer wrote:

> >> this package uses wrong installation pathes, please fix and test while
> >> you're at it (i don't use it):
> >>
> >>                 --includedir=/include \
> >> is wrong, whould be /usr/include. Most of these pathes are not needed,
> >> just --prefix=/usr should be enough.
> >
> >Will do.
>
> thanks alot in advance!

Done this too now.
Please have a look att the attached (broke some long lines too).


-- 
Cristian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ex.patch
Type: text/x-diff
Size: 1633 bytes
Desc: 
Url : http://busybox.net/lists/buildroot/attachments/20070708/7d362ac3/attachment-0001.bin