test(melange): missing dependency during emission with sandbox enabled

[anmonteiro]

based on the suggestion in #10286 (comment)

enabling sandboxing confirms there's a missing dependency

[anmonteiro]

not sure if this is needed anymore since #10286 fixed it.

[rgrinberg]

I think it's useful. It has the potential to find many such bugs. The only cost is in performance, so it could really be enabled everywhere else in the test suite as well.

[anmonteiro]

The only cost is in performance, so it could really be enabled everywhere else in the test suite as well.

I don't understand the implication here. This PR has a performance impact? But enabling more sandboxing will have less perf impact?

[anmonteiro]

IIRC there's also something preventing us from enabling sandboxing altogether for melange.emit because of the runtime_deps stuff?

[rgrinberg]

Sure, let me clarify. Sandboxing has a performance cost, but it makes the rules more reliable. For the test suite, that seems like a good trade-off because we are building trivial projects anyway.

Sandboxing works with runtime deps if the dependencies are specified correctly. I don't see any specific issues there.

I suppose one issue with sandboxing is that rules can often rely on it in a way that will make them break in non-sandboxed mode. E.g. -I in sandboxed mode is guaranteed only to observe the .cmi files you depend on. But in non sandboxed mode, it's more loose, so you need to be careful in your compiler.

[anmonteiro]

I'd love to run the test suite through sandboxing and see if it holds up for Melange. Is there a way to enable sandboxing for the entire test suite other than e.g. setting the environment variable in the dune file?

[rgrinberg]

Maybe you could use the setenv field in the env stanza to turn it on for melange tests? (setenv DUNE_SANDBOX symlink)

[anmonteiro]

I'd rather merge #10312. Just need to disable sandboxing in that odoc test.