enter_chroot: fix spurious resolv.conf warning [chromiumos/platform/crosutils : master]

2 views
Skip to first unread message

Mike Frysinger (Code Review)

unread,
May 16, 2012, 3:25:24 PM5/16/12
to Brian Harring, Zdenek Behan, Chris Sosa, Ryan Cui, Mike Frysinger
Hello Brian Harring, Zdenek Behan,

I'd like you to do a code review. Please visit

https://gerrit.chromium.org/gerrit/22845

to review the following change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................

enter_chroot: fix spurious resolv.conf warning

When bootstrapping for the first time, files in chroot/etc/ might not
exist, so we can't run `find` on them. This manifests itself currently
by spitting out the warning on all initial sdk boots:
find: `.../chroot/etc/resolv.conf': No such file or directory

People can find this confusing and cause sheriffs to waste time on the
wrong thing, so rework the code to avoid this.

BUG=None
TEST=`cros_sdk --delete ; cros_sdk` no longer warns about resolv.conf

Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
---
M sdk_lib/enter_chroot.sh
1 file changed, 10 insertions(+), 6 deletions(-)


git pull ssh://gerrit.chromium.org:29418/chromiumos/platform/crosutils refs/changes/45/22845/1
--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>
Gerrit-Reviewer: Zdenek Behan <zbe...@chromium.org>

Brian Harring (Code Review)

unread,
May 16, 2012, 6:09:53 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Brian Harring has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File sdk_lib/enter_chroot.sh
Line 143
Any reason this wasn't just modified to suppress stderr?

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment

Mike Frysinger (Code Review)

unread,
May 16, 2012, 6:17:34 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Mike Frysinger has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File sdk_lib/enter_chroot.sh
Line 143
i didn't want to silence other errors

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>
Gerrit-Reviewer: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Zdenek Behan <zbe...@chromium.org>

Brian Harring (Code Review)

unread,
May 16, 2012, 6:33:13 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Brian Harring has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File sdk_lib/enter_chroot.sh
Line 159: if ! cmp /${file} ${FLAGS_chroot}/${file} &> /dev/null; then
So... we're suppressing the output of this, but above, explicitly touching/chowning. Any reason that's not just folded into the next line (the cp) in some fashion?

Aka, have an updates array, then sudo_multi that?

The touch/chown bits from above frankly feel hackish, rather than fixing this loop- hence the pushback.

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>

Mike Frysinger (Code Review)

unread,
May 16, 2012, 7:57:34 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Mike Frysinger has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File sdk_lib/enter_chroot.sh
Line 159: if ! cmp /${file} ${FLAGS_chroot}/${file} &> /dev/null; then
these should be silenced, but for a different reason: if the source files go unreadable/disappear on us. unlikely, but possible, considering /etc/resolv.conf can be managed by host daemons.

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>

Brian Harring (Code Review)

unread,
May 16, 2012, 8:23:56 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Brian Harring has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: Looks good to me, approved

Meh. don't like it, but you've countered my points each time, so on with it... ;)

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>

Mike Frysinger (Code Review)

unread,
May 16, 2012, 11:03:55 PM5/16/12
to Mike Frysinger, Zdenek Behan, Brian Harring
Mike Frysinger has posted comments on this change.

Change subject: enter_chroot: fix spurious resolv.conf warning
......................................................................


Patch Set 1: Verified; Ready

--
To view, visit https://gerrit.chromium.org/gerrit/22845
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83f892e325e63e682aeb370a9dfc33e284e059d2
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/crosutils
Gerrit-Branch: master
Gerrit-Owner: Mike Frysinger <vap...@chromium.org>
Gerrit-Reviewer: Brian Harring <ferr...@chromium.org>
Reply all
Reply to author
Forward
0 new messages