Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 321457 - net-dns/bind 9.6.2_p2/9.7.0_p2: emerge --config fails in case there is more than one uncommented CHROOT variable
Summary: net-dns/bind 9.6.2_p2/9.7.0_p2: emerge --config fails in case there is more t...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Server (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Konstantin Arkhipov (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-25 14:41 UTC by Duncan
Modified: 2010-06-05 10:18 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Duncan 2010-05-25 14:41:21 UTC
This bug was discovered while testing for bug #321071.  It's the first time I've seen it as I've been running a chrooted bind for years and I'm only (re)testing the ebuild's emerge --config now, so it's likely in previous versions as well.

A portion of my /etc/conf.d/named file (only one line changed from the default, but the result IS a double assignment, second overwriting the first, in bash at least):

# If you wish to run bind in a chroot, run:
# emerge --config =<bind-version>
# and un-comment the following line.
# You can specify a different chroot directory but MAKE SURE it's empty.
CHROOT="/chroot/dns"
CHROOT="/m/cbind"

I have both CHROOT assignments in there for easier etc-update maintenance.  Only the second one will show as a diff, and it'll be an added line, thereby making the etc-update resolution for that line (keep it) clearer.

This works with bash as the second assignment overwrites the first, but the following sed line occurs in the ebuild in both pkg_config and pkg_postinst:

CHROOT=$(sed -n 's/^[[:blank:]]\?CHROOT="\([^"]\+\)"/\1/p' /etc/conf.d/named 2>/dev/null)

With that sed assignment, CHROOT ends up looking like this:

CHROOT="/chroot/dns /m/cbind"

That plays all sorts of havoc with the rest of pkg_config, and of course fails the [[ -d ${CHROOT} ]] in pkg_postinst as well, tho there the only result is the lack of an ewarn, so it's not /that/ bad.

If the answer is "just stop doing that with your config file", I suppose I can live with that, but of course the ideal is a somewhat more robust ebuild, so it's up to you whether this is a WONTFIX or not.
Comment 1 Duncan 2010-05-25 15:14:41 UTC
Ugh!  What's worse, even if I #-comment the original CHROOT="/chroot/dns" assignment, the last bit of pkg_config un-comments it, giving me two active assignments again!

So I can't even comment the original assignment and put in my own, as it gets uncommented as soon as I run emerge --config again (tho the config works, because it's not uncommented until the end, I just can't rerun it, as I am for testing, because it's uncommented with the first config run, and then screws stuff up again with the second!)

So something's clearly gotta be done (I take back the "just stop doing that with the config file" bit, because it's not me doing it!), because it's not simply the double assignment, but it's the --config itself creating the double assignment, when I had one of them commented!
Comment 2 Christian Ruppert (idl0r) gentoo-dev 2010-06-02 19:13:03 UTC
Both sed's are somehow problematic

Line 291:
CHROOT=$(sed -n 's/^[[:blank:]]\?CHROOT="\([^"]\+\)"/\1/p' /etc/conf.d/named 2>/dev/null)
because it will get both values in this case.

and line 333 as well:
sed -i 's/^# \?\(CHROOT.*\)$/\1/' /etc/conf.d/named 2>/dev/null
because it will uncomment _all_ commented CHROOT variables as well

I'll try to find a good solution.
Comment 3 Duncan 2010-06-02 22:36:16 UTC
(In reply to comment #2)
> Both sed's are somehow problematic
> 
> Line 291:
> CHROOT=$(sed -n 's/^[[:blank:]]\?CHROOT="\([^"]\+\)"/\1/p' /etc/conf.d/named
> 2>/dev/null)
> because it will get both values in this case.

> and line 333 as well:
> sed -i 's/^# \?\(CHROOT.*\)$/\1/' /etc/conf.d/named 2>/dev/null
> because it will uncomment _all_ commented CHROOT variables as well

After trying to come up with a decently reliable and safe way to add an uncommented CHROOT setting to the conf.d/named file  if there isn't one, I've come to the conclusion that we're doing the wrong thing in that case.  So, first of all, I'd suggest changing the "If you like to run bind in chroot" bit of the postinst to the following:

einfo "If you'd like to run bind in a chroot AND this is a new"
einfo "install OR your bind doesn't already run in a chroot:"
einfo
einfo "1) Uncomment and set the CHROOT variable in /etc/conf.d/named."
einfo "2) Run \`emerge --config '=${CATEGORY}/${PF}'\`"

This change in policy would accomplish two things:

1) As emerge --config is standard (if not often used) ebuild functionality, it's conceivable a user could try emerge --config bind, indending to have the config checked or some such, NOT intending to have it setup a bind chroot, especially if the CHROOT variable isn't set in the config file.  This would allow us to trigger only if the CHROOT variable is already set (and to bail if it's not set, with appropriate instructions to set it if a chroot is what's intended), indicating the user /wants/ a chrooted named.

2) It'd release us from trying to set the CHROOT variable properly and safely, in a config file which the user may have already made unpredicted modifications to.  This is Gentoo.  Having the user set a single variable to indicate intent before running the automated config script isn't the end of the world, and is far safer than trying to programatically edit a file the user may have already modified in unpredictable ways.

With that change:

There's nothing like bash to parse bash.  What about something like this to replace line 291-295 (the first sed and following [ -z ] if/then), plus the if/then grep and sed of lines 331-334?

CHROOT=$(source /etc/conf.d/named; echo ${CHROOT})

[[ $CHROOT ]] || {
    eerror "This config script is designed to automate setting up"
    eerror "a chrooted bind/named.  To do so, please first uncomment"
    eerror "and set the CHROOT setting in /etc/conf.d/named."
    die "Unset CHROOT"
}                                                   

The first line parses uses bash to parse CHROOT, thus eliminating any issues with multiple settings, etc.  The result is then tested, with a warning and bail if it's not set.  Can't get much simpler or safer than that. =:^)
Comment 4 Christian Ruppert (idl0r) gentoo-dev 2010-06-03 17:04:06 UTC
I like your idea so I've just fixed the pkg_config part in CVS, it should be available in a few minutes/hours.
Please re-test later (Note: there is no revision bump, you'll have to re-emerge bind as soon as you have received the changes.).
Thanks!
Comment 5 Duncan 2010-06-04 23:49:18 UTC
Works great, here! =:^)

Just a couple instructional loose ends to tie up.

* You missed the postinst einfo change in comment #3, as the current einfo says )now incorrectly) that a default chroot is used if the CHROOT variable isn't set.  I also changed the English form slightly, as what was there read awkwardly to me.  Of course, this is simply suggested wording (repeating from comment #3):

einfo "If you'd like to run bind in a chroot AND this is a new"
einfo "install OR your bind doesn't already run in a chroot:"
einfo "1) Uncomment and set the CHROOT variable in /etc/conf.d/named."
einfo "2) Run \`emerge --config '=${CATEGORY}/${PF}'\`"

* I originally missed that a similar change needs to be made to the chroot comment in the default /etc/conf.d/named file, as the uncomment now needs to be done BEFORE emerge --config is run.  Suggested wording:

# If you wish to run bind in a chroot:
# 1) un-comment the CHROOT= assignment, below. You may use
#    a different chroot directory but MAKE SURE it's empty.
# 2) run: emerge --config =<bind-version>
#
# CHROOT="/chroot/dns"

Thanks for the effort you and others have put into this! I'd have never bothered with the chroot were the work not mostly done for me.  =:^)

Duncan
Comment 6 Christian Ruppert (idl0r) gentoo-dev 2010-06-05 10:18:52 UTC
Fixed in CVS, thanks :)