Use correct scope for aliases and dependencies in submodules #2810
Conversation
|
@corvusrabus Check it out! Not your fault, after all, I completely missed it in review too, but thought you would be interested. For some reason a couple days ago after merging #2672 I was lying in bed and thought "Huh I wonder if recipes in submodules can access variables from their submodule." and sure enough they couldn't. |
|
It would be nice if we could skip evaluating modules whose scopes we don't need, i.e., because we aren't executing any recipes from those modules. However, since we eventually want to be able to import variables from other modules, we would then run into a situation where we would need to analyze imports to determine which modules we'd need to evaluate, which sounds like a bit of a headache. |
|
I'm going to merge this now since I want this bug fixed, and worry about skipping unneeded module scope evaluation later, since it's an optimization that we can do at any time. |
Damn sorry about that. I'm just trying to get involved a bit but I'm unaware of all of just's features :(. I didn't even know about variables. |
|
@corvusrabus No worries at all! It's something which is easy to miss. If I had written the feature myself, I think I could have easily not written a test which exercised variables in submodules. |
#2632 introduced a bug whereby running an alias pointing to a recipe in a submodule would not create or use the correct scope. #2672 created another way to hit the bug, this time by running a dependency in a submodule.
Both
just aandjust bcause a runtime error when trying to evaluatex, which isn't defined.Depending on a recipe in a submodule hasn't made it into a release yet, but aliasing a recipe in a submodule has, and I'm surprised the bug hasn't been reported.
This PR evaluates the assignment scope of all submodules, and runs recipes with the scope of the submodule that contains it.
One thing I'm not sure about, which is why it's still a draft, is whether or not we should only evaluate scopes of modules containing recipes that we're actually going to run.
We could go through all directly invoked recipes and all transitive dependencies, collect the set of their submodules, and then only evaluate assignments for those submodules.