[kteam-tools,v2,5/7] git-build-kernel: squelch output noise when there is no debian/debian.env

Message ID 1533249635-9483-6-git-send-email-kamal@canonical.com
State New
Headers show
Series
  • git-build-kernel: source pkg features
Related show

Commit Message

Kamal Mostafa Aug. 2, 2018, 10:40 p.m.
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 git-build-kernel/git-build-kernel | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andy Whitcroft Aug. 6, 2018, 3:33 p.m. | #1
On Thu, Aug 02, 2018 at 03:40:33PM -0700, Kamal Mostafa wrote:
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  git-build-kernel/git-build-kernel | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-build-kernel/git-build-kernel b/git-build-kernel/git-build-kernel
> index a683ad4..3c19587 100755
> --- a/git-build-kernel/git-build-kernel
> +++ b/git-build-kernel/git-build-kernel
> @@ -68,7 +68,7 @@ GITSHASHORT="`git log -1 --pretty=%h ${GITBRANCH%%refs/heads/} --`"
>  }
>  
>  ### Get the var "DEBIAN=debian.master" from debian/debian.env
> -eval `git show $GITBRANCH:debian/debian.env | grep DEBIAN=`
> +eval `git show $GITBRANCH:debian/debian.env 2>&- | grep DEBIAN=`

I am not used to seeing stderr being closed.  That could lead to
applications failing when they attempt to report an error.  Which in the
general case might lead to unexpected behaviour.  I am wondering why
this is not 2>/dev/null which feels like a more normal approach.

-apw
Kamal Mostafa Aug. 6, 2018, 4:52 p.m. | #2
On Mon, Aug 06, 2018 at 04:33:47PM +0100, Andy Whitcroft wrote:
> On Thu, Aug 02, 2018 at 03:40:33PM -0700, Kamal Mostafa wrote:
> > Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> > ---
> >  git-build-kernel/git-build-kernel | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/git-build-kernel/git-build-kernel b/git-build-kernel/git-build-kernel
> > index a683ad4..3c19587 100755
> > --- a/git-build-kernel/git-build-kernel
> > +++ b/git-build-kernel/git-build-kernel
> > @@ -68,7 +68,7 @@ GITSHASHORT="`git log -1 --pretty=%h ${GITBRANCH%%refs/heads/} --`"
> >  }
> >  
> >  ### Get the var "DEBIAN=debian.master" from debian/debian.env
> > -eval `git show $GITBRANCH:debian/debian.env | grep DEBIAN=`
> > +eval `git show $GITBRANCH:debian/debian.env 2>&- | grep DEBIAN=`
> 
> I am not used to seeing stderr being closed.  That could lead to
> applications failing when they attempt to report an error.  Which in the
> general case might lead to unexpected behaviour.  I am wondering why
> this is not 2>/dev/null which feels like a more normal approach.

I'll change it to 2>/dev/null before committing.

 -Kamal

P.S. I close stderr all the time and have never observed any app having
a problem with that.  If any did, I'd call that a bug in that app.

Patch

diff --git a/git-build-kernel/git-build-kernel b/git-build-kernel/git-build-kernel
index a683ad4..3c19587 100755
--- a/git-build-kernel/git-build-kernel
+++ b/git-build-kernel/git-build-kernel
@@ -68,7 +68,7 @@  GITSHASHORT="`git log -1 --pretty=%h ${GITBRANCH%%refs/heads/} --`"
 }
 
 ### Get the var "DEBIAN=debian.master" from debian/debian.env
-eval `git show $GITBRANCH:debian/debian.env | grep DEBIAN=`
+eval `git show $GITBRANCH:debian/debian.env 2>&- | grep DEBIAN=`
 [ -z "$DEBIAN" ] && DEBIAN=debian
 
 
@@ -84,7 +84,7 @@  SRCPKG=$(git show $GITBRANCH:$DEBIAN/changelog | sed 's/^\(.*\) (.*/\1/;q')
 CHROOT="$DISTRO"
 CHROOT="${CHROOT%-proposed}"
 [ $do_source_pkg = 1 ] && {
-    eval `git show $GITBRANCH:debian/debian.env | grep SOURCE_PACKAGE_CHROOT=`
+    eval `git show $GITBRANCH:debian/debian.env 2>&- | grep SOURCE_PACKAGE_CHROOT=`
     [ -z "$SOURCE_PACKAGE_CHROOT" ] || CHROOT="$SOURCE_PACKAGE_CHROOT"
 }
 if [ "$CHROOT" = "UNRELEASED" -o -z "$CHROOT" ]