NetBSD Problem Report #48375

From kim@talisker.gw.com  Sat Nov  9 15:41:34 2013
Return-Path: <kim@talisker.gw.com>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 98CF8A619F
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  9 Nov 2013 15:41:34 +0000 (UTC)
Message-Id: <20131109142626.C4263DD439@talisker.gw.com>
Date: Sat,  9 Nov 2013 09:26:26 -0500 (EST)
From: kim@netbsd.org (Kimmo Suominen)
Reply-To: kim@netbsd.org (Kimmo Suominen)
To: gnats-bugs@gnats.NetBSD.org
Subject: etckeeper fails to run with /bin/sh
X-Send-Pr-Version: 3.95

>Number:         48375
>Category:       pkg
>Synopsis:       etckeeper fails to run with /bin/sh
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 09 15:45:00 +0000 2013
>Last-Modified:  Sun Nov 10 16:30:00 +0000 2013
>Originator:     Kimmo Suominen
>Release:        NetBSD 6.1.1
>Organization:
	Internet
>Environment:
System: NetBSD talisker.gw.com 6.1.1 NetBSD 6.1.1 (XEN3_DOMU) amd64
Architecture: x86_64
Machine: amd64
>Description:
	Etckeeper fails to run, because /bin/sh considers unsetting
	a non-existing variable fatal and as "set -e" is in effect
	exits.

	Using bash (from pkgsrc) would appear to work.

	Alternatively avoid unsetting variables that are not set.
	This might be a problematic strategy as additional etckeeper
	scripts might also fail.

	Looking at the scripts distributed with etckeeper, it looks
	like all other uses are "safe," only unsetting previously set
	variables.

	./commit.d/30darcs-add
	./init.d/60darcs-deleted-symlinks
	./init.d/70vcs-add
	./pre-commit.d/30store-metadata
>How-To-Repeat:
	talisker:/etc# /bin/sh -x /usr/pkg/bin/etckeeper init
	+ set -e
	+ '[' -z '' ']'
	+ 'ETCKEEPER_CONF_DIR=/etc/etckeeper'
	+ 'conf=/etc/etckeeper/etckeeper.conf'
	+ '[' -e '/etc/etckeeper/etckeeper.conf' ']'
	+ '.' '/etc/etckeeper/etckeeper.conf'
	+ VCS=bzr
	+ 'GIT_COMMIT_OPTIONS='
	+ 'HG_COMMIT_OPTIONS='
	+ 'BZR_COMMIT_OPTIONS='
	+ 'DARCS_COMMIT_OPTIONS=-a'
	+ 'HIGHLEVEL_PACKAGE_MANAGER=pkg_add'
	+ 'LOWLEVEL_PACKAGE_MANAGER=pkg_install'
	+ 'PUSH_REMOTE='
	+ unset 'GIT_WORK_TREE'
	Exit 1
>Fix:
	Using bash seems like a safe choice.

Index: Makefile
===================================================================
RCS file: /cvsroot/pkgsrc/sysutils/etckeeper/Makefile,v
retrieving revision 1.3
diff -u -r1.3 Makefile
--- Makefile	26 Jun 2013 14:26:47 -0000	1.3
+++ Makefile	9 Nov 2013 14:21:07 -0000
@@ -14,9 +14,8 @@

 WRKSRC=		${WRKDIR}/etckeeper
 USE_LANGUAGES=	# none
-USE_TOOLS+=	sed perl:run
+USE_TOOLS+=	bash:run sed perl:run

-NO_CONFIGURE=	yes
 NO_BUILD=	yes
 AUTO_MKDIRS=	yes

@@ -40,6 +39,8 @@
 				${REAL_ROOT_USER} ${REAL_ROOT_GROUP} 755
 .endfor

+REPLACE_BASH+=		etckeeper
+
 SUBST_CLASSES+=		config
 SUBST_STAGE.config=	pre-install
 SUBST_FILES.config+=	etckeeper

>Audit-Trail:
From: Aleksej Saushev <asau@inbox.ru>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
Date: Sun, 10 Nov 2013 05:38:30 +0400

 > 	Etckeeper fails to run, because /bin/sh considers unsetting
 > 	a non-existing variable fatal and as "set -e" is in effect
 > 	exits.

 PR bin/48312.

 The fix should be pulled into supported versions rather.


 -- 
 BCE HA MOPE!

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
Date: Sun, 10 Nov 2013 10:45:40 +0700

     Date:        Sun, 10 Nov 2013 01:45:01 +0000 (UTC)
     From:        Aleksej Saushev <asau@inbox.ru>
     Message-ID:  <20131110014501.74ECBA61B1@mollari.NetBSD.org>

   |  PR bin/48312.
   |  The fix should be pulled into supported versions rather.

 That is not a solution to the PR in the Subject.   By all means, fix
 (already done) and pull up (not sure on that) the fix, but pkgsrc neither
 can nor should rely upon that, it needs to work even on older broken
 NetBSDs.

 If there was a sh bug on Solaris (well, millions...) or on Darwin, or
 OpenBSD, ... we wouldn't even comtemplate just saying "upgrade to a fixed
 shell", we'd find some kind of a workaround, and incorporate that, one way
 or another (that is, in the bootstrap for that system, or in the package
 Makefile or patches or ...) into pkgsrc, so it works on NetBSD 6.0 as
 promised, not just 6.0++

 On the other hand, requiring bash (just using ksh would probably work, and
 not require a dependency, but even that is overboard) for this trivial
 problem is overkill.

 If there's a variable that's being unset, without ever being set, just
 install a patch to set the thing before the unset happens, and no more
 problems, that is, if

 	unset x

 is the issue, just make it be

 	x= ; unset x

 and we're done.  (Don't omit the ';' or we get the weird special effects
 of assignments on special builtins in the sh definition).

 The fact that this problem has (apparently) been in the shell so long,
 and caused so few problems suggests that this isn't going to require
 zillions of patches all over the place, just the off one, here and there.

 kre

From: Kimmo Suominen <kim@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
Date: Sun, 10 Nov 2013 09:59:17 +0200

 --527f3cd5_2d1d5ae9_f8a7
 Content-Type: text/plain; charset="utf-8"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline

 On Sun, 10 Nov 2013 at 5:50, Robert Elz wrote:
 > If there's a variable that's being unset, without ever being set, just
 > install a patch to set the thing before the unset happens, and no more
 > problems, that is, if
 > 
 > unset x
 > 
 > is the issue, just make it be
 > 
 > x= ; unset x
 > 
 > and we're done.

 I considered that, but thought that it is not the best approach due to the architecture of etckeeper: each etckeeper command actually maps to a directory, which contains any number of scripts to execute. 3rd-party scripts might have adopted the same style of unsetting things that have not been set, and would fail.

 On top of that, the failure mode is very unfriendly: the command just exits with an exit code of 1. There is no error message, even.

 + Kimmo



 --527f3cd5_2d1d5ae9_f8a7
 Content-Type: text/html; charset="utf-8"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: inline


                 <div style=3D=22font-family: Helvetica; font-size: 13px;=22=
 ><span style=3D=22color: rgb(160, 160, 168);=22>On Sun, 10 Nov 2013 at 5:=
 50, Robert Elz wrote:</span></div>
                 <blockquote type=3D=22cite=22 style=3D=22border-left-styl=
 e:solid;border-width:1px;margin-left:0px;padding-left:10px;=22>
                     <span><div>If there's a variable that's being unset, =
 without ever being set, just</div><div> install a patch to set the thing =
 before the unset happens, and no more</div><div> problems, that is, if</d=
 iv><div> </div><div> 	unset x</div><div> </div><div> is the issue, just m=
 ake it be</div><div> </div><div> 	x=3D ; unset x</div><div> </div><div> a=
 nd we're done.</div></span></blockquote><div>I considered that, but thoug=
 ht that it is not the best approach due to the architecture of etckeeper:=
  each etckeeper command actually maps to a directory, which contains any =
 number of scripts to execute. 3rd-party scripts might have adopted the sa=
 me style of unsetting things that have not been set, and would fail.</div=
 ><div><br></div><div>On top of that, the failure mode is very unfriendly:=
  the command just exits with an exit code of 1. There is no error message=
 , even.</div><div><br></div><div>+ Kimmo</div><div>&nbsp;</div>
                 =20
                 <div>
                     <br>
                 </div>

 --527f3cd5_2d1d5ae9_f8a7--

From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
Date: Sun, 10 Nov 2013 15:17:04 +0100

 On Sun, Nov 10, 2013 at 03:50:01AM +0000, Robert Elz wrote:
 > The following reply was made to PR pkg/48375; it has been noted by GNATS.
 > 
 > From: Robert Elz <kre@munnari.OZ.AU>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
 > Date: Sun, 10 Nov 2013 10:45:40 +0700
 > 
 >      Date:        Sun, 10 Nov 2013 01:45:01 +0000 (UTC)
 >      From:        Aleksej Saushev <asau@inbox.ru>
 >      Message-ID:  <20131110014501.74ECBA61B1@mollari.NetBSD.org>
 >  
 >    |  PR bin/48312.
 >    |  The fix should be pulled into supported versions rather.
 >  
 >  That is not a solution to the PR in the Subject.   By all means, fix
 >  (already done) and pull up (not sure on that) the fix, but pkgsrc neither
 >  can nor should rely upon that, it needs to work even on older broken
 >  NetBSDs.

 I completely disagree with you. Other Operating Systems are completely
 irrelevant in this context. Consider the latest point release whatever
 we try to support and to a degree the matching release branch. I find
 the attitude of "everything must work around the bugs in .0" very
 unproductive, especially if the result is an accumulation of workarounds
 in random programs that likely will never get cleaned up and just create
 technical debt.

 Joerg

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/48375: etckeeper fails to run with /bin/sh
Date: Sun, 10 Nov 2013 23:25:54 +0700

     Date:        Sun, 10 Nov 2013 14:20:01 +0000 (UTC)
     From:        Joerg Sonnenberger <joerg@britannica.bec.de>
     Message-ID:  <20131110142001.81E2BA61B0@mollari.NetBSD.org>

   |  Consider the latest point release whatever
   |  we try to support and to a degree the matching release branch.

 There is no release of NetBSD with the fix for this bug in it, so for
 this particular PR, it really makes no difference.

   |  I find the attitude of "everything must work around the bugs in .0" very
   |  unproductive,

 That's only because you're thinking of pkgsrc as a part of NetBSD (which is
 also why you're claiming what is done for other OS's as irrelevant).
 Pkgsrc claims to be portable.   That means it needs to run on all kinds of
 systems, with all of their bugs - including older NetBSDs (as far back as they
 remain supported - by pkgsrc.)

   | especially if the result is an accumulation of workarounds
   | in random programs that likely will never get cleaned up and just create
   | technical debt.

 First, it is entirely possible that bugs in NetBSD utilities might be
 duplicated in other systems, so when it is as trivial (& safe) to work
 around as this one is, adding the workaround makes sense just as a defensive
 measure so the same problem doesn't need to be investigated and solved
 again on some other system.

 Second, it is entirely reasonable to ask the originators of the package
 to accept a patch to handle this problem, doing so would be easy for them
 and essentially cost free.   So, it need not even necessarily require keeping
 a patch in pkgsrc for very long.

 kre

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.