<@U0CHHN4F4> do you have a moment to review a simp...
# scripting
p
@ilya.chernikov do you have a moment to review a simple PR? 🙏 Scripting: add reproducer for stale cache if using imports
👀 1
i
Hi @Piotr KrzemiƄski The PR is fine, but so far, I don't see a point in having a "wrong" test there without any fix. Could you please explain why you want to have it without a fix? Or you're not intending to propose a fix and want to help only with the test?
p
it just brings us closer to solving this issue (saves work of someone that will work on fixing it), and it's a result of 2-3 evenings of my work - I think committing little pieces more frequently has a value on its own. I'm planning to work on fixing it within several days, but there's some risk I'll tackle it later
if you feel strongly about not merging it as is, it's enough for me to know from you if the test looks good - then I'll include it in the fixing PR
i
I would very much prefer to have a complete PR with the fix. Merging a PR is not a one-click action, according to our process. I would rather mention it in the MR and the issue so the one who will work on the fix can merge the PR along. And of course, if you plan to do it yourself - it's even better, we'll appreciate it very much.
👍 1
I'll comment on the PR in GH.
p
got it, I thought merging a PR is a cheap process for you. Thanks!
i
I commented on it there. Thank you for your efforts!
thank you color 1
p
@ilya.chernikov I had a quick look at how it could be solved, and I need some guidance + agreeing on the approach. Currently the cache looks only at the source of the script being executed. It doesn't know about the imports because, as I assume, the script hasn't yet been compiled. So we have a chicken-egg problem. I propose the following approach: have the first pass that will analyze just the
@file:Import()
annotations and follow them. Once done, we can either inline the source code of the imported scripts so that the cache can calculate the hash using all files including imported, or have an object that will contain references to the imported files and build the hash iteratively using all files. After this step, proceed to regular compilation. Does it make any sense?
i
I think this will lead to a big logic duplication. It seems to me that it would be better to move the call to
withScriptCompilationCache
down the callstack, around the end of
ScriptJvmCompilerImplsKt.compileImpl
, where the actual compilation is called with already fully refined configurations. (We'll need to extend
withScriptCompilationCache
to accept multiple sources and configurations too. Theoretically, partial caching should be also possible at this place, but we can postpone it for now.) This approach would also handle a potential problem with clearing the
.m2
cache.
👀 1