On Mon, 2010-02-22 at 15:59 -0800, Markus Roberts wrote:
> Brice (and anyone else who wants to kibitz/opine) --
Since you sent it to me only, I'm not sure anyone beside me will
answer :-)
I'm CC:ing puppet-dev so that other people can chime in.
> I'm interested in your thoughts about some of the edge cases involving
> hashes that I'm having trouble reconciling with futures. Consider (to
> get the conversation started at least) the following:
>
> (markus) ~/projects/puppet (hash-testing)> cat scope_test_4.pp
>
> $a = [a,b]
> $h = {a=>b}
>
> class c1 {
> notice "2: a = $a, h = $h"
> $a += [c,d]
> $h[c] = d
> notice "3: a = $a, h = $h"
> # At this point these are not allowed:
> # $a += [e,f] # Additional array modification
> # $h[a] = z # Changing a hash element
> # but:
> $h[e] = f
> notice "4: a = $a, h = $h"
> $h += {a => z}
This one should clearly be forbidden. It shouldn't be possible to
rewrite an hash value of an existing key.
> notice "5: a = $a, h = $h"
> }
>
> notice "1: a = $a, h = $h"
> include c1
> notice "6: a = $a, h = $h"
>
> (markus) ~/projects/puppet (hash-testing)> puppet scope_test_4.pp
> notice: Scope(Class[main]): 1: a = ab, h = ab
> notice: Scope(Class[c1]): 2: a = ab, h = ab
> notice: Scope(Class[c1]): 3: a = abcd, h = abcd
> notice: Scope(Class[c1]): 4: a = abcd, h = abcdef
> notice: Scope(Class[c1]): 5: a = abcd, h = azcdef
> notice: Scope(Class[main]): 6: a = ab, h = azcdef
This one looks problematic too. All modifications to h inside c1 should
have been done on a copy in the c1 scope not on the original hash.
I bet there is something wrong in my patch.
It looks like the hash instance is shared in all the scopes instead of
being duplicated (why does it work for arrays then?).
> (markus) ~/projects/puppet (hash-testing)>
>
> There are several things going on here, and I'm of two minds on at
> least half of them.
>
> * In the futures-branch I consider this array behaviour
> incorrect in that it is order dependent within c1, and aim
> for:
> $a = [a,b]
>
> class c1 {
> notice "2: a = $a"
> $a += [c,d]
> notice "3: a = $a"
> }
>
> notice "1: a = $a"
> include c1
> notice "4: a = $a"
> --------------------------------------------------------
> notice: Scope(Class[main]): 1: a = ab
> notice: Scope(Class[c1]): 2: a = abcd
> notice: Scope(Class[c1]): 3: a = abcd
> notice: Scope(Class[main]): 4: a = ab
I don't see what the issue is.
To me it's exactly the same as writing:
$a = "test"
class c1 {
$a = "toto ${a} titi"
}
include c1
That is we're shadowing the main $a in the scope of c1.
> * I'm not sure why $h[a] = z should be forbidden and $h +=
> {a=>z} allowed, nor which is "correct"
It should be forbidden. That's something I obviously missed.
> * If multiple extensions to hashes are allowed, what's the
> rationale for prohibiting them for arrays (on the assumption
> that all references that see any of the changes see all of
> them)? One possible answer is that arrays, being ordered,
> would then expose execution order.
When I originally implemented the append operator, I wanted in fact to
simplify:
$a = "string"
class newscope {
$a = "new string using ${string}"
}
Since it is possible to create a new variable only once, I used the
exact same semantic for the append operator (both for arrays and regular
variables).
Now, since it seems to be possible to extend hashes more than one time
in a definite scope, I did it wrong for them.
I'm somewhat completely undecided about what behaviour for hash is
correct or not, about mulitple appending. The conservative guy deep
inside me would tend to say that it should also be fordidden.
> * Should hashes "leak" changes to the outer scope? On the one
> hand, this is AFAIK very different from how anything else in
> puppet works; on the other it is the basis for some of the
> most compelling use cases for hashes.
I'd say they shouldn't. Variables don't do that, array don't do that,
hash sould not do that.
> There's more, but this should get us started.
Indeed :-)
Now, I have a question: let's say I'm fixing the holes you found in the
hash patch, what is the best way to submit the fixes?
Among the possibilities are:
* producing a new testing based branch, still rebased on the same
commit as now
* producing a master branch
* something else?
(choose your poison).
Thanks for your analysis of the issues,
--
Brice Figureau
Follow the latest Puppet Community evolutions on www.planetpuppet.org!
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
