Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
@ 2011-07-04 19:03 Peter Korsgaard
  2011-11-23 11:40 ` Maxime Ripard
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2011-07-04 19:03 UTC (permalink / raw)
  To: buildroot


commit: http://git.buildroot.net/buildroot/commit/?id=abf7af17e95855e9fe2535729f7d83fce603463f
branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 package/busybox/busybox.mk |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 4931b07..509c3bb 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -123,8 +123,10 @@ endef
 endif
 
 define BUSYBOX_INSTALL_LOGGING_SCRIPT
-	$(INSTALL) -m 0755 -D package/busybox/S01logging \
-		$(TARGET_DIR)/etc/init.d/S01logging
+	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
+		$(INSTALL) -m 0755 -D package/busybox/S01logging \
+			$(TARGET_DIR)/etc/init.d/S01logging; \
+	else rm -f $(TARGET_DIR)/etc/init.d/S01logging; fi
 endef
 
 # We do this here to avoid busting a modified .config in configure
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-07-04 19:03 [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled Peter Korsgaard
@ 2011-11-23 11:40 ` Maxime Ripard
  2011-11-23 15:05   ` Peter Korsgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2011-11-23 11:40 UTC (permalink / raw)
  To: buildroot

Hi Peter,

Looking at the init scripts installation stuff, I found this, which is
confusing to me.

There seems to be a glitch here.

AFAIK, when I proposed that name, we renamed all the init scripts for
logging daemons to S01logging.

But this combined with this commit leads to erasing the init scripts for
other daemons when logging is not enabled in busybox.

While it shouldn't happen in the common use-case, because of the
dependency we set on busybox in all the logger packages, if the user put
his init script for another logger in the skeleton, it will always
remove it, and install the default one.

Maybe the common name was not such a good idea, and that we should have
a distinct name for each init scripts. Or we revert this commit. I don't
know.

On 04/07/2011 21:03, Peter Korsgaard wrote:
> 
> commit: http://git.buildroot.net/buildroot/commit/?id=abf7af17e95855e9fe2535729f7d83fce603463f
> branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master
> 
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> ---
>  package/busybox/busybox.mk |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 4931b07..509c3bb 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -123,8 +123,10 @@ endef
>  endif
>  
>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
> -	$(INSTALL) -m 0755 -D package/busybox/S01logging \
> -		$(TARGET_DIR)/etc/init.d/S01logging
> +	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
> +		$(INSTALL) -m 0755 -D package/busybox/S01logging \
> +			$(TARGET_DIR)/etc/init.d/S01logging; \
> +	else rm -f $(TARGET_DIR)/etc/init.d/S01logging; fi
>  endef
>  
>  # We do this here to avoid busting a modified .config in configure


-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-11-23 11:40 ` Maxime Ripard
@ 2011-11-23 15:05   ` Peter Korsgaard
  2011-11-23 21:41     ` Peter Korsgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2011-11-23 15:05 UTC (permalink / raw)
  To: buildroot

>>>>> "Maxime" == Maxime Ripard <maxime.ripard@free-electrons.com> writes:

 Maxime> Hi Peter,

 Maxime> Looking at the init scripts installation stuff, I found this,
 Maxime> which is confusing to me.

 Maxime> There seems to be a glitch here.

Ahh yes.

 Maxime> While it shouldn't happen in the common use-case, because of
 Maxime> the dependency we set on busybox in all the logger packages, if
 Maxime> the user put his init script for another logger in the
 Maxime> skeleton, it will always remove it, and install the default
 Maxime> one.

 Maxime> Maybe the common name was not such a good idea, and that we
 Maxime> should have a distinct name for each init scripts. Or we revert
 Maxime> this commit. I don't know.

For 2011.11 I would suggest we just remove the 2nd part (E.G. removal of
init script if syslog daemon isn't enabled) - The first part is needed
to support custom S01logging scripts with busybox (E.G. I have projects
where we log to shared memory instead of files, so you need -S option to
syslogd).

Once 2011.11 is out, then we should probably rename the files to work
around this.

I'll fix - Thanks for bringing it out.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-11-23 15:05   ` Peter Korsgaard
@ 2011-11-23 21:41     ` Peter Korsgaard
  2011-11-23 21:56       ` Maxime Ripard
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2011-11-23 21:41 UTC (permalink / raw)
  To: buildroot

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

Hi,

 Maxime> While it shouldn't happen in the common use-case, because of
 Maxime> the dependency we set on busybox in all the logger packages, if
 Maxime> the user put his init script for another logger in the
 Maxime> skeleton, it will always remove it, and install the default
 Maxime> one.

 Maxime> Maybe the common name was not such a good idea, and that we
 Maxime> should have a distinct name for each init scripts. Or we revert
 Maxime> this commit. I don't know.

 Peter> For 2011.11 I would suggest we just remove the 2nd part (E.G. removal of
 Peter> init script if syslog daemon isn't enabled) - The first part is needed
 Peter> to support custom S01logging scripts with busybox (E.G. I have projects
 Peter> where we log to shared memory instead of files, so you need -S option to
 Peter> syslogd).

 Peter> Once 2011.11 is out, then we should probably rename the files to work
 Peter> around this.

 Peter> I'll fix - Thanks for bringing it out.

Ehh, I had a closer look, and I don't see any issues:

- If sysklogd applet is enabled in busybox we install S01logging
- If "big" sysklogd is enabled we don't install anything (bug)
- If rsyslogd is enabled we install S01rsyslog

So we don't have multiple system loggers using the same file name - Did
you just mean that we should rename S01logging to something like
S01bbsysklog instead?

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-11-23 21:41     ` Peter Korsgaard
@ 2011-11-23 21:56       ` Maxime Ripard
  2011-11-23 22:16         ` Peter Korsgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2011-11-23 21:56 UTC (permalink / raw)
  To: buildroot

Le 23/11/2011 22:41, Peter Korsgaard a ?crit :
>>>>>> "Peter" == Peter Korsgaard<jacmet@sunsite.dk>  writes:
>   Maxime>  While it shouldn't happen in the common use-case, because of
>   Maxime>  the dependency we set on busybox in all the logger packages, if
>   Maxime>  the user put his init script for another logger in the
>   Maxime>  skeleton, it will always remove it, and install the default
>   Maxime>  one.
>
>   Maxime>  Maybe the common name was not such a good idea, and that we
>   Maxime>  should have a distinct name for each init scripts. Or we revert
>   Maxime>  this commit. I don't know.
>
>   Peter>  For 2011.11 I would suggest we just remove the 2nd part (E.G. removal of
>   Peter>  init script if syslog daemon isn't enabled) - The first part is needed
>   Peter>  to support custom S01logging scripts with busybox (E.G. I have projects
>   Peter>  where we log to shared memory instead of files, so you need -S option to
>   Peter>  syslogd).
>
>   Peter>  Once 2011.11 is out, then we should probably rename the files to work
>   Peter>  around this.
>
>   Peter>  I'll fix - Thanks for bringing it out.
>
> Ehh, I had a closer look, and I don't see any issues:
>
> - If sysklogd applet is enabled in busybox we install S01logging
> - If "big" sysklogd is enabled we don't install anything (bug)
> - If rsyslogd is enabled we install S01rsyslog

It is more in the case where :
   - The user has a custom S01logging script for rsyslog (also works for 
all but busybox syslog)
   - Busybox runs first, as rsyslog depends on it
   - Busybox init is not enabled, so the package removes S01logging
   - rsyslog is built, and as there is no S01logging, installs its 
default one.

This is the bug to me, even though adding a custom init script is not 
very likely.

> So we don't have multiple system loggers using the same file name - Did
> you just mean that we should rename S01logging to something like
> S01bbsysklog instead?

Well, I meant it as a solution to the case above. If we have different 
names for all of them, busybox's package will never remove another's 
init script.

Maxime

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-11-23 21:56       ` Maxime Ripard
@ 2011-11-23 22:16         ` Peter Korsgaard
  2011-11-24  8:03           ` Maxime Ripard
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2011-11-23 22:16 UTC (permalink / raw)
  To: buildroot

>>>>> "Maxime" == Maxime Ripard <maxime.ripard@free-electrons.com> writes:

 Maxime> It is more in the case where :
 Maxime>   - The user has a custom S01logging script for rsyslog (also works
 Maxime> for all but busybox syslog)
 Maxime>   - Busybox runs first, as rsyslog depends on it
 Maxime>   - Busybox init is not enabled, so the package removes S01logging
 Maxime>   - rsyslog is built, and as there is no S01logging, installs its
 Maxime> default one.

But rsyslog already installs (unconditially, which is a bug) 01rsyslog -
People would presumably have a custom S01rsyslog file if they use
rsyslog instead of a S01logging file?

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled
  2011-11-23 22:16         ` Peter Korsgaard
@ 2011-11-24  8:03           ` Maxime Ripard
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2011-11-24  8:03 UTC (permalink / raw)
  To: buildroot

On 23/11/2011 23:16, Peter Korsgaard wrote:
>>>>>> "Maxime" == Maxime Ripard <maxime.ripard@free-electrons.com> writes:
> 
>  Maxime> It is more in the case where :
>  Maxime>   - The user has a custom S01logging script for rsyslog (also works
>  Maxime> for all but busybox syslog)
>  Maxime>   - Busybox runs first, as rsyslog depends on it
>  Maxime>   - Busybox init is not enabled, so the package removes S01logging
>  Maxime>   - rsyslog is built, and as there is no S01logging, installs its
>  Maxime> default one.
> 
> But rsyslog already installs (unconditially, which is a bug) 01rsyslog -
> People would presumably have a custom S01rsyslog file if they use
> rsyslog instead of a S01logging file?

Oh yes, you're right, rsyslog has S01rsyslog. Well it was my intent to
do so at first then, but I didn't submitted anything to do it that way.

Sorry about that :)

Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-11-24  8:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-04 19:03 [Buildroot] [git commit] busybox: only install S01logging if syslogd applet is enabled Peter Korsgaard
2011-11-23 11:40 ` Maxime Ripard
2011-11-23 15:05   ` Peter Korsgaard
2011-11-23 21:41     ` Peter Korsgaard
2011-11-23 21:56       ` Maxime Ripard
2011-11-23 22:16         ` Peter Korsgaard
2011-11-24  8:03           ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox