FIX 496 : Add support for CodeSource URL for GoloClassLoader #497
Conversation
|
I think we should take the opportunity of this PR to refactor code loading in the commands. Indeed, almost every commands scan some directory for golo files and load/compile them, each in its own way. The |
| * @return the class matching the Golo module defined in the source. | ||
| * @throws GoloCompilationException if either of the compilation phase failed. | ||
| */ | ||
| public synchronized Class<?> load(String goloSourceFilename, InputStream sourceCodeInputStream, URL sourceCodeLocation) throws GoloCompilationException { |
There was a problem hiding this comment.
This method is copy/past from the previous one. You should refactor the previous one to call this one with a null location.
Or even better, as stated in the main comments, refactor the method to only take a CodeSource and derive the name and input stream from it, and remove the old one.
| @@ -78,7 +78,7 @@ public void execute() throws Throwable { | |||
| } | |||
| } else if (file.getName().endsWith(".golo")) { | |||
| try (FileInputStream in = new FileInputStream(file)) { | |||
| Class<?> loadedClass = loader.load(file.getName(), in); | |||
| Class<?> loadedClass = loader.load(file.getName(), in, file.toURI().toURL()); | |||
There was a problem hiding this comment.
Build the CodeSource instance here (see the proposed load refactoring). This will allows to construct a Certificate[] from command line options if we ever need to.
BTW, the file scanning facility should be extracted.
There was a problem hiding this comment.
@yloiseau what do you mean by "extracted"?
There was a problem hiding this comment.
There should be a mode generic method that scan (recursively) a directory and return golo files, maybe in the form of a CodeSource object, or Path with a method to convert a Path into a CodeSource
There was a problem hiding this comment.
see the "extract method" refactoring...
| @@ -78,7 +78,7 @@ private void loadOtherGoloFiles(GoloClassLoader loader, Path basedir, Path scrip | |||
| try (InputStream is = Files.newInputStream(path)) { | |||
| Path filename = path.getFileName(); | |||
| if (filename != null) { | |||
| return loader.load(filename.toString(), is); | |||
| return loader.load(filename.toString(), is, path.toUri().toURL()); | |||
|
Alright, I've seen your comments but am not sure of when I'll be able to work on this, though it seems to be pretty easy, so if anyone wants to do this instead of me feel free to do so. |
|
Since it's not a critical bug/feature, you can take some times to implement it 😉 |
This PR aims to fix #496 where it is impossible to get a .golo file's location at runtime. This uses the same mechanism as regular .jar and .classes in Java, which is registering a ProtectionDomain and CodeSource.
This PR adds a
loadmethod toGoloClassLoaderthat takes one argument more than the already present load method, which is a URL. This URL represents the original location of the .golo file.Both the CLI "golo golo" and "golosh" were changed to use the new method. The old method was kept to ensure backwards-compatibility and still provide a way to load golo files without providing its source. This could also be useful in the future if you guys want to use Certificates or other security features of ProtectionDomain with GoloClassLoader.