Conversation
jurgenvinju
commented
Mar 10, 2025
- replaced use of lib scheme and removed superfluous code that created big classpaths for the JVM which were then again constructed inside of the REPL by PathConfig calling into mvn
- fixed issues
- set to the right SNAPSHOT version of rascal
- started working on rewrite towards using main processes of tutor, compiler and checker
…big classpaths for the JVM which were then again constructed inside of the REPL by PathConfig calling into mvn
Watch out the snapshot dependency on rascal also needs to change to "unshaded" again
…piler and checker
…ascal dependency of the current project, or otherwise the rascal dependency of the maven plugin itself
…y maven compile warnings
DavyLandman
left a comment
There was a problem hiding this comment.
Looks good, but I think the stale detection logic needs a bit more configurability.
I do like how it cleans up the code.
| command.add("-verbose"); | ||
| } | ||
|
|
||
| return new ProcessBuilder(command).inheritIO().start(); |
There was a problem hiding this comment.
The plexus-tools from maven has some dedicated tooling to invoke nexted commands: https://codehaus-plexus.github.io/plexus-utils/apidocs/org/codehaus/plexus/util/cli/Commandline.html
can we use that? (and let it take care of some of the messy work?)
There was a problem hiding this comment.
noted. What messy work are you referring to? This is pretty straightforward stuff.
|
|
||
| if (name.endsWith(".rsc")) { | ||
| return Set.of( | ||
| new File(targetDir, new File(file.getParentFile(), "$" + name.substring(0, name.length() - ".rsc".length()) + ".tpl").getPath()) |
There was a problem hiding this comment.
in the case of the tutor or the repackager, this is not the right stale detection.
There was a problem hiding this comment.
those (tutor and packager) do not use this code; but to make this more clear I will add a parameter to generalize it (in case we do use it in the future)
|
|
||
| return builder.toString().substring(1); | ||
| } | ||
| public GenerateSourcesUsingRascalMojo(String mainClass, String skipTag) { |
There was a problem hiding this comment.
we also use generate sources for non rascal projects, so that looks like we'll loose that capability?
There was a problem hiding this comment.
No this works as before. The RascalShell main still interprets its first commandline parameter as the module to load and run.
There was a problem hiding this comment.
I will test if it still works as before.
There was a problem hiding this comment.
command.add(getRascalRuntime().toString());
command.add(mainClass);
if (mainModule != null && !mainModule.isEmpty()) {
command.add(mainModule);
}
| } | ||
| // starts the processes asynchronously | ||
| for (int i = 1; i < chunks.size(); i++) { | ||
| processes.add(runMain(verbose, chunks.get(1), srcs, libs, tmpGeneratedSources.get(i), tmpBins.get(i))); |
There was a problem hiding this comment.
Shouldn't this be chunks.get(1) but i?
And also, we should put the bin folder of the preCheck phase on the lib path of the parallel runner. so that it doesn't have to start all over again and recheck List etc.
There was a problem hiding this comment.
also: since runMain uses inheritIO this will start "biting" each other.
It would be better to have the process allocate it's own streams, and then copy from them in a streaming way, making sure to lock before we write to our own streams. (like was happening in the original code, but it was doing this inside of the same JVM)
There was a problem hiding this comment.
- chunks.get(i): nice catch!
- streams: yes that would be cool. We could also greatly simplify this and merge the output when everybody is done, without interleaving every line. I think I'd prefer that; it is much simpler and it is much easier to read.
There was a problem hiding this comment.
yes, with the downside that while the typecheck is happening, you have no feedback. so it looks like it's stuck.
There was a problem hiding this comment.
And also, we should put the bin folder of the preCheck phase on the lib path of the parallel runner. so that it doesn't have to start all over again and recheck List etc
nice catch!
…feature can be activated
…hers do their work silently