Review fix: fix incorrect assumption in IncrementalLauncher#saveCache

[LakshyAAAgrawal]

Fix #3404
I am not sure if this should treated as a bug or not, since essentially the source jar file is not present/visible to spoon.

[monperrus]

Thanks, does it successfully reproduce the buggy behavior described in #3404? Which assertion is failing?

[LakshyAAAgrawal]

Generated by this testcase:

spoon.support.compiler.SnippetCompilationError: The import soot cannot be resolved at src/test/resources/incremental/saveCacheIssue3404/A.java:3
	at spoon.test.model.IncrementalLauncherTest.testSaveCacheIssue3404(IncrementalLauncherTest.java:241)

Reported by user:

spoon.support.compiler.SnippetCompilationError: The import org.apache.ibatis cannot be resolved 
at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.reportProblemsWhenCompiling(JDTBasedSpoonCompiler.java:590) 
at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.compile(JDTBasedSpoonCompiler.java:160)
at spoon.IncrementalLauncher.saveCache(IncrementalLauncher.java:267)

The error is raised in the saveCache() function call in both cases and pertains to SnippetCompilationError regarding unresolved import from missing jar file.

In the test case, if the line containing the actual jar file in sourceClasspath is uncommented, the test passes.

Set<String> sourceClasspath = Collections.singleton("./src/test/resources/incremental/saveCacheIssue3404/soot_jar/sootclasses-trunk-jar-with-dependencies.jar");
//Set<String> sourceClasspath = Collections.EMPTY_SET;

[monperrus]

Thanks for the explanation, it seems that it is the same bug.

I suspect that the fix consists of ensuring that saveCache works in NOCLASSPATH mode. It should be easy.

Would you make a try and find a fix?

Thanks!

[LakshyAAAgrawal]

Sure. I will push the fix once I am able to solve the issue. Thanks for the suggestion.

[monperrus]

OK, thanks! --Martin

[LakshyAAAgrawal]

I am still working on the issue. Here are some observations:

I tested the "Launcher" class with the same testcase and it works and also compiles. Then further I tried to understand where might "IncrementalLauncher" be setting an expected classpath. I tried to call factory.getEnvironment().setNoClasspath(true) from various different places - The testcase, beginning of constructor call, end of constructor call, saveCache, etc. But it still does not work in NOCLASSPATH mode. Printing the value of "getArguments().getString("cpmode")" which is used to initialize the super Launcher class reports NOCLASSPATH mode. So it looks like it should be working in NOCLASSPATH mode but it does not. I suspect that some changes will need to be made in the IncrementalLauncher constructor since unlike Launcher class, the input resources for IncrementalLauncher are added in the constructor call itself.

I will continue to work on the issue and any suggestions are welcome.

[monperrus]

Thanks for the analysis.

I suspect that some changes will need to be made in the IncrementalLauncher constructor since unlike Launcher class, the input resources for IncrementalLauncher are added in the constructor call itself.

Agree!

[LakshyAAAgrawal]

I have tried different changes in the incrementalLauncher constructor as well as saveCache, but I have not been able to solve the issue. I might be missing a very basic point, and I would be very happy if someone could please guide me on how to progress with the issue.

I have tried the following things:

1. Compare the values within "processArguments" for the constructor call for "Launcher" as well as "IncrementalLauncher". All the argument values for both the classes are same. So that couldn't have caused a difference.
2. Setting "getEnvironment().setNoClasspath(true);" at the beginning of the constructor call.
3. Setting factory.getEnvironment().setNoClasspath(true) under "forceRebuild" branch.
4. Setting oldFactory.getEnvironment().setNoClasspath(true) under (not "forceRebuild") branch.
5. "getModelBuilder().getFactory().getEnvironment().setNoClasspath(true);" before call to compile.

Reading IncrementalLauncher code, I am not able to determine where might the behaviour difference between "IncrementalLauncher" and "Launcher" occurs.

The line launcher.buildModel() calls modelBuilder.build() which is marked saying it should be only called once for a given factory. I executed a test for "Launcher" class which was succefully building the model in NOCLASSPATH mode. I called buildmodel for it and then ran compile, and that raised the same error. So I think that calling launcher.buildModel() before saveCache() might be causing an issue, so I have removed the line temprorarily.

I have also explored a few other options but none yielded any result. Could someone suggest a direction to explore?

[monperrus]

Hi @LakshyAAAgrawal finally looked at this.

The problem is that method saveCache calls compile, which obviously does not work in noclasspath.

Pushed to your branch, see the fix here.

Thanks a lot for the failing test case, it was super useful.

[nharrand]

Thanks a lot for the PR!