Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add description of Morganey index format #307

Merged
merged 5 commits into from
Nov 29, 2016
Merged

Conversation

rexim
Copy link
Member

@rexim rexim commented Nov 27, 2016

Close #303

@rexim
Copy link
Member Author

rexim commented Nov 27, 2016

Remark for myself: I think it also worth mentioning that Morganey index is not required to actually load Morganey modules, it is only needed if one wants to have the REPL autocompletion.

@keddelzz
Copy link
Collaborator

it is only needed if one wants to have the REPL autocompletion.

I think it's not even needed for auto-completion. It would be possible to search through all directories and jar-files on the classpath to create (and cache) the morganey-index once on the start of morganey.

@rexim
Copy link
Member Author

rexim commented Nov 27, 2016

@keddelzz how? Do you suggest to break the ClassLoader abstraction and traverse jars and folders manually?

@keddelzz
Copy link
Collaborator

Do you suggest to break the ClassLoader abstraction and traverse jars and folders manually?

Yes. In my opinion is the abstraction provided by class loaders is not good enough. It simply lacks the feature to traverse the classpath. This can be done by breaking the abstraction and manually traversing it.

@rexim
Copy link
Member Author

rexim commented Nov 27, 2016

@keddelzz so you suggest to completely scrap #222 and everything that've been discussed there and in the corresponding subtasks?

@rexim
Copy link
Member Author

rexim commented Nov 27, 2016

I mean, as far as I know ClassLoader abstraction IS the classpath mechanism of JVM. Breaking the ClassLoader abstraction effectively means abandoning the JVM classpath mechanism and doing everything manually.

@keddelzz
Copy link
Collaborator

@rexim i'm sorry, i think we might have a misunderstanding. I don't suggest to discard #222. We should use the classpath-mechanism of the jvm. But the classloader lacks the ability to actually scan through the contents of the classpath.

However it is possible to do that manually. We could ask the classloader to give us all (class-)paths of the current program. We can traverse them and build the morganey-index. Once it is built we continue to use the class-loader to load the resources (Class.getResource and Class.getResourceAsStream).

@rexim
Copy link
Member Author

rexim commented Nov 27, 2016

@keddelzz, as far as I know classpath may contain completely arbitrary mediums. Not only jars and folders. Breaking ClassLoader abstraction in such way locks us on jars and folders only. Adding a new type of medium for a Morganey module container will be an additional work from our side. Do you believe that jars and folders is enough for everyone? :)

@keddelzz
Copy link
Collaborator

Adding a new type of medium for a Morganey module container will be an additional work from our side.

I'm completely aware of that.

Do you believe that jars and folders is enough for everyone? :)

Probably not. Someone will need a different source, because there's always one, who need a little extra 😄.


Because I missed the stream today I have no idea how the morganey-index will be used in the future. Will we ask the class-loader to give us all known indices as resources and use their information for auto-completion and module-loading?

# Morganey Index

Morganey index is a file called `morganey-index` that is located at
the root of Morganey module container. Morganey module container is a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that you rephrase that as "the root of every Morganey module container".

Morganey index is a file called `morganey-index` that is located at
the root of Morganey module container. Morganey module container is a
place that is discoverable via JVM classpath mechanism (jar, folder,
etc) and contains Morganey modules. For example, here is the structure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also let's add a clarification here: "Morganey runtime can load multiple modules at once, and each of them should contain its own Index file".

@@ -0,0 +1,32 @@
# Morganey Index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you should add a link to this file from some documentation root (probably from the Readme file). Or will you do it later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root is gonna be introduce in the scope of #107

@ForNeVeR
Copy link
Member

And now regarding your discussion here, guys. I think that it's practical enough to go with the Morganey Index for now. I don't think that we need some additional reflected functionality to enumerate classpath (although I believe that's possible and can be done through Google Reflection library).

  1. There could be a custom ClassLoader that cannot be enumerated in a sensible way (for example, HTTP class loader).
  2. Instead of coupling Morganey with existing file system and JAR ClassLoaders I propose that we later add an index generator functionality. So the guy who uses the custom ClassLoader (or adapts it for use with Morganey) will be responsible for messing with indices.

Overall, I don't think that we'll really need some custom class loader support in any near future, but I don't think we should hardly restrict Morganey with the standard ones. So, we shouldn't add any sort of classpath scanner if we doubt it will be easy to couple with custom class loaders.

Also, we could actually implement a morganey-index generator for jar files without Morganey index — let's just file a separated issue about that. But remember, @keddelzz, we're open for discussion. If you think that the whole Morganen Index thing is not what we want, then please let's discuss it further.

@rexim
Copy link
Member Author

rexim commented Nov 28, 2016

@keddelzz

Because I missed the stream today I have no idea how the morganey-index will be used in the future. Will we ask the class-loader to give us all known indices as resources and use their information for auto-completion and module-loading?

Yeah, that was exactly the intend. We even created a test project to double check that getting all of the available indices works exactly as we expect: https://github.com/tsoding/jvm-resources-experiment

@ForNeVeR

Also, we could actually implement a morganey-index generator for jar files without Morganey index — let's just file a separated issue about that. But remember, @keddelzz, we're open for discussion. If you think that the whole Morganen Index thing is not what we want, then please let's discuss it further.

I like this idea. Module loading is performed via ClassLoader abstraction only. If one needs autocompletion he or she have to make sure that a corresponding morganey-index is available via ClassLoader. On top of that we have a "separate entity" that scans classpath for jars and folders and tries to generate indices if they are not present their. For crazy stuff like an HTTP class loader, the user have to make sure that the morganey-index is already available their, because our "index generator" doesn't know anything about HTTP. That means even if you have some really unusual module container medium, it is still possible to have REPL autocompletion if you generate morganey-index yourself.

Sounds perfect for me! You guys are awesome! :) Let's create and issue for that. I'm gonna do that.

@rexim
Copy link
Member Author

rexim commented Nov 28, 2016

So I think now I understand what you meant in #307 (comment) Sorry for the misunderstanding. :) It's a pretty good idea. Thanks!

@keddelzz
Copy link
Collaborator

There could be a custom ClassLoader that cannot be enumerated in a sensible way (for example, HTTP class loader).

@ForNeVeR , I didn't thought of such an unusual class-loader, thank you for reminding me.

If you think that the whole Morganen Index thing is not what we want, then please let's discuss it further.

I do think that we want the morganey-index after reading your comment. I didn't think of the weird class-loaders people can define. After all the morganey-index will be (probably) a lot faster and is much more elegant than scanning the class-path. My intention was to remind you guys, that a custom class-path-traverser might simplify the work for a morganey library author.

@rexim rexim assigned rexim and unassigned ForNeVeR and keddelzz Nov 28, 2016
@rexim rexim assigned ForNeVeR and keddelzz and unassigned rexim Nov 28, 2016
@rexim
Copy link
Member Author

rexim commented Nov 28, 2016

Fixed the remarks.

@rexim
Copy link
Member Author

rexim commented Nov 29, 2016

@ForNeVeR @keddelzz thank you guys for the review!

@rexim rexim merged commit ed62bcd into master Nov 29, 2016
@rexim rexim deleted the 303-morganey-index-format branch November 29, 2016 17:04
@rexim rexim removed the in progress label Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants