On Mon, May 6, 2013 at 8:03 PM, Dave Reisner <d@falconindy.com> wrote:
On Mon, May 06, 2013 at 07:49:48PM +0200, Jouke Witteveen wrote:
On Mon, May 6, 2013 at 7:36 PM, Dave Reisner <d@falconindy.com> wrote:
On Mon, May 06, 2013 at 07:23:58PM +0200, Jouke Witteveen wrote:
You are right. This (arrays in Bash) is pretty tricky. I believe it is fixed now. If so, the fix will land in netctl 1.1.
Thanks, - Jouke
The problem is that you're declaring it initially as a string, and then expecting it to be an array. You simply cannot interchange the types without encountering unwanted behavior like this.
I'd like to keep the possibility of having declarations like this in a profile:
Address='1.2.3.4/5'
Although Address is treated like an array internally, this works. As far as I can tell it is relatively safe to treat non-empty strings as 1-element arrays.
I think this is the wrong thing to do. You need to enforce types if you're consuming raw bash as configuration. You're telling people that k='foo' is valid, but k='foo bar' is not and suddenly they must use k=(foo bar). This is bizzare.
It is not documented that k='foo' is valid. Accepting it is a means of lowering user complaints. In Bash, it is quite natural to accept strings as single element arrays: declare -a X X=foo is a single element array, and if X is a string for x in "${X[@]}"; do ... works as expected. Also, if X is an array, $X is its first element.
Note that you're still doing this in src/netctl.in:112. I'll also point out that your -v check allows people to create profiles that do not bind to any interfaces since it checks for the definition and not the existence of a value -- are you sure you want this?
Regarding src/netctl.in:112, the variable is set to a non-empty string (in src/lib/globals:load_profile, it is actually verified that $Interface is non-empty). By the above safety assumption, this should be acceptable. The current behavior is to bind to the Interface by default. By explicitly setting BindsToInterfaces, this behavior can be altered. Indeed, it is possible to do
BindsToInterfaces=''
in a profile, which will most likely screw things up (it will instruct systemd to bind to a device without a name). However, I consider this a usage error. The current configuration allows setting BindsToInterfaces to a possibly empty array as well as specifying it correctly by a non-empty string. Those are the uses that I would like to support. If a better way exists, I'd be happy to hear about it :-).
And based on what you said above, you don't believe this is a double standard? If someone were to adhere to the fact that BindsToInterfaces is an array and not a string, then BindsToInterfaces=() is a valid way of declaring this as empty and netctl will respect that.
BindsToInterfaces=() is valid and meaningful. What the code doesn't do is give an error if BindsToInterfaces (or any other variable presumed to be an array) is not an array. There is no code to enforce compatibility, only lack of code to enforce strictness. Line 112 could be changed to [[ -v BindsToInterfaces || -z $BindsToInterfaces ]] || \ BindsToInterfaces=( "$Interface" ) But this would change nothing in any *supported* case. In fact, the '-z' part is debatable, because it pays attention to a situation that is not supported anyway. Type checking is ugly in Bash and wouldn't yield anything. Instead, all variables that are supposed to be arrays could be declared as such a priori, which makes assigning strings to them 'do the right thing'. In the BindsToInterfaces case, this would then reintroduce the problems of deciding whether interface binding is desired at all (currently: not specified = bind to Interface; specified as empty array = no binding; otherwise: bind to all 'components') To summarize my view: : The current code treats arrays as arrays without type checking : There is no code to make wacky shit possible : As a side effect, strings as single element arrays are possible I believe the entire discussion is only concerned with src/netctl.in:112 :-P. - Jouke