Emitting warnings broke caching

[calebzulawski]

8 broke file caching.

It's broken in a particularly confusing way: with --log debug (and verified with strace) I can see invocations to the compiler, however, with --log info there are no tasks spawned. Odd, because those compiler invocations are strictly within tasks.

[afranchuk]

I was concerned about that and almost brought it up, but I figured you had tested it :D But now I'm facepalming because I think the issue is a bit obvious (to me) and I should have suggested it. I think it would be fixed by changing warnings = ... to String :warnings = .... That change will at least have the same evaluation semantics as pre-#8.

[afranchuk]

Though I'm not really sure, as technically the cache function should evaluate the returned value fully. But try that out and see what happens.

[calebzulawski]

I tested it, but I only checked to see that warnings were emitted on incremental builds... That was my first suspicion, but it doesn't seem to fix it. I also tried a few brute force things like id ~set=$obj $warnings, no luck. Seeing exec running without a task ever being spawned is particularly concerning.

[afranchuk]

Actually the problem is likely that indexing is eagerly evaluated in id calculation (i.e. another id eval issue :roll_eyes:). The |>:stderr isn't scoped to anything in that block, so it can be eagerly evaluated. Change from ... |>:stderr to bind (...) (index stderr) or std:index {:stderr} = ...; $stderr | String:from | String:trim. Really ugly :( This is more evidence that there needs to be some more sanity brought to id evaluation.

[afranchuk]

These id evaluation things are closely related to the function purity stuff, since e.g. cache could be considered impure since there are definitely side effects (which might prevent the value from being evaluated at all!) so id evaluation shouldn't eagerly do anything with the arguments (nominally).

[afranchuk]

I've seen similarly confusing events occur (specifically with execs within task/cache), and it's really not easy to reason about at all. It definitely needs to be improved.

[calebzulawski]

That was exactly it! I came to a similar conclusion that it was related specifically to stderr. This does seem like a significant footgun--especially since exec was making it into id evaluation without being "gated" by task limits (maybe this is also the cause for #6?).

[afranchuk]

It could totally be the cause of #6, I definitely saw that in the past (and that's to what I was referring in the comment there, I had to make changes that adjusted the id evaluation bit).