From 8ea7d4cf8456b9661995d49fe01075f62708aa0b Mon Sep 17 00:00:00 2001 From: James Shubin Date: Wed, 31 Jan 2024 20:25:58 -0500 Subject: [PATCH] engine: graph, util: Restore send/recv behaviour A regression in 4b0cdf912372a6da76fea3130b6a5b3b59c428e7 caused the basic send/recv functionality to break for simple scenarios. This was due to inadequate testing, and a partial misunderstanding of the situation. New testing should hopefully catch more cases, but send/recv and compile-time checks are still not as complete as is probably possible. --- engine/graph/sendrecv.go | 57 +++++++++++++------ engine/util/util.go | 12 ++-- .../TestAstFunc3/send-recv-0.txtar | 31 ++++++++++ .../TestAstFunc3/send-recv-1.txtar | 18 ++++++ .../TestAstFunc3/sendrecv-simple1.txtar | 32 +++++++++++ .../TestAstFunc3/sendrecv-simple2.txtar | 15 +++++ .../TestAstFunc3/sendrecv-simple3.txtar | 44 ++++++++++++++ 7 files changed, 186 insertions(+), 23 deletions(-) create mode 100644 lang/interpret_test/TestAstFunc3/send-recv-0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/send-recv-1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/sendrecv-simple1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/sendrecv-simple2.txtar create mode 100644 lang/interpret_test/TestAstFunc3/sendrecv-simple3.txtar diff --git a/engine/graph/sendrecv.go b/engine/graph/sendrecv.go index 73a4a950..0ea79222 100644 --- a/engine/graph/sendrecv.go +++ b/engine/graph/sendrecv.go @@ -165,12 +165,35 @@ func SendRecv(res engine.RecvableRes, fn RecvFn) (map[engine.RecvableRes]map[str //orig := value1 dest := value2 // save the o.g. because we need the real dest! - // For situations where we send a variant to the resource! - for kind1 == reflect.Interface || kind1 == reflect.Ptr { + // NOTE: Reminder: obj1 comes from st and it is the *Sends + // struct which contains whichever fields that resource sends. + // For example, this might be *TestSends for the Test resource. + // The receiver is obj2 and that is actually the resource struct + // which is a * and which gets it's fields directly set on. + // For example, this might be *TestRes for the Test resource. + //fmt.Printf("obj1(%T): %+v\n", obj1, obj1) + //fmt.Printf("obj2(%T): %+v\n", obj2, obj2) + // Lastly, remember that many of the type incompatibilities are + // caught during type unification, and so we might have overly + // relaxed the checks here and something could slip by. If we + // find something, this code will need new checks added back. + + // Here we unpack one-level, and then leave the complex stuff + // for the Into() method below. + // for kind1 == reflect.Interface || kind1 == reflect.Ptr // wrong + // if kind1 == reflect.Interface || kind1 == reflect.Ptr // wrong + // for kind1 == reflect.Interface // wrong + if kind1 == reflect.Interface { value1 = value1.Elem() // un-nest one interface kind1 = value1.Kind() } - for kind2 == reflect.Interface || kind2 == reflect.Ptr { + + // This second block is identical, but it's just accidentally + // symmetrical. The types of input structs are different shapes. + // for kind2 == reflect.Interface || kind2 == reflect.Ptr // wrong + // if kind2 == reflect.Interface || kind2 == reflect.Ptr // wrong + // for kind2 == reflect.Interface // wrong + if kind2 == reflect.Interface { value2 = value2.Elem() // un-nest one interface kind2 = value2.Kind() } @@ -180,20 +203,19 @@ func SendRecv(res engine.RecvableRes, fn RecvFn) (map[engine.RecvableRes]map[str // obj.Logf("Recv(%s) has %v: %v", type2, kind2, value2) //} - // i think we probably want the same kind, at least for now... - if kind1 != kind2 { - e := fmt.Errorf("send/recv kind mismatch between %s: %s and %s: %s", v.Res, kind1, res, kind2) - err = errwrap.Append(err, e) // list of errors - continue - } + // Skip this check in favour of the more complex Into() below... + //if kind1 != kind2 { + // e := fmt.Errorf("send/recv kind mismatch between %s: %s and %s: %s", v.Res, kind1, res, kind2) + // err = errwrap.Append(err, e) // list of errors + // continue + //} - // if the types don't match, we can't use send->recv - // FIXME: do we want to relax this for string -> *string ? - if e := TypeCmp(value1, value2); e != nil { - e := errwrap.Wrapf(e, "type mismatch between %s and %s", v.Res, res) - err = errwrap.Append(err, e) // list of errors - continue - } + // Skip this check in favour of the more complex Into() below... + //if e := TypeCmp(value1, value2); e != nil { + // e := errwrap.Wrapf(e, "type mismatch between %s and %s", v.Res, res) + // err = errwrap.Append(err, e) // list of errors + // continue + //} // if we can't set, then well this is pointless! if !dest.CanSet() { @@ -230,7 +252,8 @@ func SendRecv(res engine.RecvableRes, fn RecvFn) (map[engine.RecvableRes]map[str // mutate the struct field dest with the mcl data in fv if e := types.Into(fv, dest); e != nil { - e := errwrap.Wrapf(e, "bad dest %s.%s", v.Res, v.Key) + // runtime error, probably from using value res + e := errwrap.Wrapf(e, "mismatch: %s.%s (%s) -> %s.%s (%s)", v.Res, v.Key, kind1, res, k, kind2) err = errwrap.Append(err, e) // list of errors continue } diff --git a/engine/util/util.go b/engine/util/util.go index f404cfc2..0907dcea 100644 --- a/engine/util/util.go +++ b/engine/util/util.go @@ -192,16 +192,16 @@ func StructFieldCompat(st1 interface{}, key1 string, st2 interface{}, key2 strin return fmt.Errorf("can't interface the recv") } - // If we're sending _from_ an interface... + // If we're sending _from_ an interface... (value res `any` field) if kind1 == reflect.Interface || kind1 == reflect.Ptr { // TODO: Can we do more checks instead of only returning early? return nil } - // If we're sending _to_ an interface... - //if kind2 == reflect.Interface { - // // TODO: Can we do more checks instead of only returning early? - // return nil - //} + // If we're sending _to_ an interface... (value res `any` field) + if kind2 == reflect.Interface || kind2 == reflect.Ptr { + // TODO: Can we do more checks instead of only returning early? + return nil + } if kind1 != kind2 { return fmt.Errorf("field kind mismatch between %s and %s", kind1, kind2) diff --git a/lang/interpret_test/TestAstFunc3/send-recv-0.txtar b/lang/interpret_test/TestAstFunc3/send-recv-0.txtar new file mode 100644 index 00000000..e8525b8f --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/send-recv-0.txtar @@ -0,0 +1,31 @@ +-- main.mcl -- +exec "exec0" { + cmd => "echo this is stdout && echo this is stderr 1>&2", # to stdout && stderr + shell => "/bin/bash", +} + +file ["/tmp/command-output", "/tmp/command-stdout", "/tmp/command-stderr",] { + state => $const.res.file.state.exists, +} + +Exec["exec0"].stdout -> File["/tmp/command-stdout"].content +Exec["exec0"].stderr -> File["/tmp/command-stderr"].content +# XXX: The Content line can be swapped randomly, create test to allow either. +# XXX: In the meantime, we skip including it. +#Exec["exec0"].output -> File["/tmp/command-output"].content +# Field: file[/tmp/command-output].Content = "this is stdout\nthis is stderr\n" +# Field: file[/tmp/command-output].Content = "this is stderr\nthis is stdout\n" +-- OUTPUT -- +Edge: exec[exec0] -> file[/tmp/command-stderr] # exec[exec0] -> file[/tmp/command-stderr] +Edge: exec[exec0] -> file[/tmp/command-stdout] # exec[exec0] -> file[/tmp/command-stdout] +Field: exec[exec0].Cmd = "echo this is stdout && echo this is stderr 1>&2" +Field: exec[exec0].Shell = "/bin/bash" +Field: file[/tmp/command-output].State = "exists" +Field: file[/tmp/command-stderr].Content = "this is stderr\n" +Field: file[/tmp/command-stderr].State = "exists" +Field: file[/tmp/command-stdout].Content = "this is stdout\n" +Field: file[/tmp/command-stdout].State = "exists" +Vertex: exec[exec0] +Vertex: file[/tmp/command-output] +Vertex: file[/tmp/command-stderr] +Vertex: file[/tmp/command-stdout] diff --git a/lang/interpret_test/TestAstFunc3/send-recv-1.txtar b/lang/interpret_test/TestAstFunc3/send-recv-1.txtar new file mode 100644 index 00000000..5b68b0df --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/send-recv-1.txtar @@ -0,0 +1,18 @@ +-- main.mcl -- +test "send" { + sendvalue => "this is hello", # sends on key of `hello` + # we also secret send on key of `answer` the int value of 42 + + Meta:autogroup => false, +} + +test "recv" { + expectrecv => ["anotherstr",], # expecting to recv on these keys! + + Meta:autogroup => false, +} + +# must error, incompatible types +Test["send"].answer -> Test["recv"].anotherstr +-- OUTPUT -- +# err: errUnify: cannot send/recv from test[send].answer to test[recv].anotherstr: field kind mismatch between int and string diff --git a/lang/interpret_test/TestAstFunc3/sendrecv-simple1.txtar b/lang/interpret_test/TestAstFunc3/sendrecv-simple1.txtar new file mode 100644 index 00000000..ac9ec61e --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/sendrecv-simple1.txtar @@ -0,0 +1,32 @@ +-- main.mcl -- +# send/recv of value1.any into test.msg works! +value "value1" { + any => "i am value1", +} +test "test1" { + sendvalue => "hello from test", + + Meta:autogroup => false, +} +value "value2" { + any => "", # gets value from send_value above +} +value "value3" { + any => 0, # gets 42 +} +Value["value1"].any -> Test["test1"].anotherstr +Test["test1"].hello -> Value["value2"].any +Test["test1"].answer -> Value["value3"].any +-- OUTPUT -- +Edge: test[test1] -> value[value2] # test[test1] -> value[value2] +Edge: test[test1] -> value[value3] # test[test1] -> value[value3] +Edge: value[value1] -> test[test1] # value[value1] -> test[test1] +Field: test[test1].AnotherStr = "i am value1" +Field: test[test1].SendValue = "hello from test" +Field: value[value1].Any = "i am value1" +Field: value[value2].Any = "hello from test" +Field: value[value3].Any = 42 +Vertex: test[test1] +Vertex: value[value1] +Vertex: value[value2] +Vertex: value[value3] diff --git a/lang/interpret_test/TestAstFunc3/sendrecv-simple2.txtar b/lang/interpret_test/TestAstFunc3/sendrecv-simple2.txtar new file mode 100644 index 00000000..29048dfe --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/sendrecv-simple2.txtar @@ -0,0 +1,15 @@ +-- main.mcl -- +# send/recv of value1.any into test.msg works! +value "value1" { + any => "i am value1", +} +print "print1" { + #msg => "", # gets value from send_value above +} +Value["value1"].any -> Print["print1"].msg +-- OUTPUT -- +Edge: value[value1] -> print[print1] # value[value1] -> print[print1] +Field: print[print1].Msg = "i am value1" +Field: value[value1].Any = "i am value1" +Vertex: print[print1] +Vertex: value[value1] diff --git a/lang/interpret_test/TestAstFunc3/sendrecv-simple3.txtar b/lang/interpret_test/TestAstFunc3/sendrecv-simple3.txtar new file mode 100644 index 00000000..e9f4a38c --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/sendrecv-simple3.txtar @@ -0,0 +1,44 @@ +-- main.mcl -- +# send/recv of value1.any into test.msg works! +value "value1" { + any => "i am value1", +} +test "test1" { + sendvalue => "hello from test", + + Meta:autogroup => false, +} +value "value2" { + any => "", # gets value from send_value above +} +value "value3" { + # XXX: This works because this value gets overwritten, but it really + # should get caught at type unification if possible. It is caught at + # runtime and we allow it for now since we want the escape hatch with + # the `any` types for the moment. The error looks like: + # print[print1]: Error: could not SendRecv: + # mismatch: value[value3].any (ptr) -> print[print1].msg (string): + # cannot Into() 42 of type int into string + any => "NOPE", # gets 42 +} +print "print1" {} + +Value["value1"].any -> Test["test1"].anotherstr +Test["test1"].hello -> Value["value2"].any +Test["test1"].answer -> Value["value3"].any +Value["value3"].any -> Print["print1"].msg +-- OUTPUT -- +Edge: test[test1] -> value[value2] # test[test1] -> value[value2] +Edge: test[test1] -> value[value3] # test[test1] -> value[value3] +Edge: value[value1] -> test[test1] # value[value1] -> test[test1] +Edge: value[value3] -> print[print1] # value[value3] -> print[print1] +Field: test[test1].AnotherStr = "i am value1" +Field: test[test1].SendValue = "hello from test" +Field: value[value1].Any = "i am value1" +Field: value[value2].Any = "hello from test" +Field: value[value3].Any = 42 +Vertex: print[print1] +Vertex: test[test1] +Vertex: value[value1] +Vertex: value[value2] +Vertex: value[value3]