plugin.yml for other plugins gets shadowed by cljminecraft

[CmdrDats]

Thanks to the classloader sharing - this needs to be looked into.

[ghost]

what do you mean by this ? seems to be working ok with memorystone, but ofc I might be missing something by other plugins you mean only clojure related plugins which depend on cljminecraft I presume

EDIT: an instance of ClojurePlugin is created for every plugin (ie. cljminecraft and memorystone each have a different one) - if this is even relevant:D

[ghost]

ok i got it now :)

[ghost]

hmm, weird, I think I solved this problem... but let me make sure

[ghost]

ok aparently 3 things:

1. delete .\cb\plugins\memorystone\config.yml (so it will be recreated on first run) with these contents: repl.enabled: false
2. make sure onEnable executes loadClojureFile(name+"/core"); only for cljminecraft not for any child plugins(like memorystone) - because without 3. below it will fail
3. remove class-loader-of: cljminecraft from child plugins (like memorystone)

But something must be done still to load(maybe from cljminecraft's clojure code?) memorystone's core script because it fails on disable with calling unbound fn exception (although i defined it)

[ghost]

I think this line in cljminecraft/core is actually loading memorystone's core but only the start method ? (EDIT: without having to load the script from java prior to this) when-let [resolved (resolve (symbol (str (.getName plugin) ".core/start")))] that's why no errors onEnable, but only onDisable Thoughts?

EDIT: this is why I'm hoping that loading memorystone/core from cljminecraft's clojure code instead of java maybe might work? or maybe we don't have to? we only need onDisable with same resolve

EDIT2: works like a charm so far, for .core/start and .core/stop with the same when-let and no config.yml shadowing oh wait, now I wonder if you meant config.yml or plugin.yml or both, and if this fix actually fixes both or only config.yml ...

I'd submit a pull request but I've so many changes which you wouldn't want (in the java file(s), not any in clojure files though)

[ghost]

ok looks like that was an epic fail on my part lol, for one, I didn't even rename my methods from enable-plugin to start for example and then even if I do, the when-let would never resolve that because the script wasn't loaded grrr, so in effect it doesn't call start from memorystone, meh

[CmdrDats]

The class-loader-of is quite fundamental (and it's a pull request I actually made to Bukkit for the clj-minecraft plugin :) ), All the plugins need to live under the same class-loader as the clj-minecraft plugin so that the clojure classloader can find all the resources required.

The plugin.yml is safe because it gets loaded by Bukkit before it starts doing any classpath resolution or anything, it's just the config.yml.

My thinking is that we simply provide a function for default config for clojure plugins, like a

(config {:repl.enabled false})

that would create the plugin.yml in the correct, circumventing the problem altogether.

On another note, I think you have a point in that the cljminecraft.core should have a start function too and only in there fire up the repl. That circumvents the problem of having to specify repl.enabled false in each plugin, which is a pain in the neck.

Thanks for all your feedback - very much appreciated!

I would like to pull in your work when you're happy with the way it works for you, since I would encourage us both to be working off a common codebase as much as possible and I value the work you've put into it. If there are things that you've added that really shouldn't be in this project, I'll be sure to either let you know or just refactor it myself :)

[ghost]

I definitely remember seeing your pull request for class-loader-of quite some time ago. Ok cool, thanks for your replies... I kinda took a break for a sec because of my failing with the classloader and the config.yml ...

My thinking is that we simply provide a function for default config for clojure plugins, like a

about this, is this only for defaults ? I mean, if the file already exists, does it work when querying its values? as I've seen it doesn't, I mean, I manually edited the memorystone repl value to false in the existing config.yml and it still seen it as true, but I might be wrong about this - I'll have to recheck (I think I only tried it about 3 times but still, unsure)

that would create the plugin.yml in the correct, circumventing the problem altogether.

here you mean config.yml ? else I don't get it :)

[ghost]

I was wrong: already existing config.yml is read correctly, only the defaults are borrowed from cljminecraft's config.yml(which is inside its .jar not the existing config.yml)

[ghost]

hmm, I am able to load memorystone without having class-loader-of: cljminecraft in it's plugin.yml but I have to manually specify the classpath to the memorystone*.jar file and of course I have to use cljminecraft's classloader :)) but it's from inside java code at least

but the good thing is, that as far as I can tell , memorystone doesn't get cljminecraft's defaults this way thoughts?

EDIT: cljminecraft: 05:55:45 [INFO] cljminecraft.core:Starting repl on host: 0.0.0.1, port 4005 cljminecraft's in .jar config.yml

repl:
  host: 0.0.0.0
  port: 4005
  enabled: true

cljminecraft's on disk config.yml (manually modified by je):

repl:
  host: 0.0.0.1
  port: 4005
  enabled: true 

memorystone: 05:55:45 [INFO] cljminecraft.core:Repl options: null 4006 false memorystone's in .jar config.yml repl.enabled: false memorystone's on disk config.yml (manually modified by je):

repl:
  port: 4006
  enabled: false 

EDIT2: I should mention(just in case it's not clear from above) that normally (with class-loader-of, aka your latest commit) it memorystone shows 0.0.0.0 instead of null, which means it took it from cljminecraft's in .jar config.yml

[CmdrDats]

Cool, thanks for looking into this - I'm definitely leaning towards providing a direct function in clojure for specifying the defaults so that the config.yml doesn't even exist in either cljminecraft or any plugins.

I certainly don't want people (especially server admins) to have to fudge around with classpaths in order to get clojure based plugins to work, it should be as simple as drag & drop, just like every other plugin, otherwise we'd be inhibiting the adoption of clojure based plugins.. it will already be a bit of a pain depending on the cljminecraft plugin, but at least the dependency mechanism is well understood.

In effect, I reckon we don't use the bukkit configuration helper functions at all, just read/write the [pluginname]/config.yml file directly and provide our own clojure functions to deal with it - we can get the same effect that way with no unexpected classpathy side effects and provide defaults directly in the code (which always ends up a better idea anyhow, in my opinion..)

[ghost]

alrighty Now, it seems to me that the reason why it gets the "wrong" config.yml from the (wrong) .jar is due to as you said using same classloader(or classpath? I notice that both cljminecraft and memorystone were on classpath in that order with that clojure classloader) so maybe in order to make sure that any child plugin loading resources from inside .jar don't get the wrong file (from the shadowing cljminecraft if exists there) I'll be pursuing this variant a bit and see how it works - after all I'm only messing around/testing stuff, I'm nowhere near as good as(or clearminded as you are) judging by all the code you've written in cljminecraft for example. There is practically no change in the child plugin except the no need to specify class-loader-of, works fine. And in cljminecraft I am able to specify automatically detected .jar file for the current(child or cljminecraft) plugin as classpath. In FactionsPlus I also opted to not including config.yml in .jar but that was another story. Technically, I believe, both variants can exist: no config.yml and no class-loader-of.

I got stuck a while on trying to get eclipse+ccw show me javadoc for the leiningen dependencies of bukkit but failed

I didn't try yet, but I wonder if something like get resource(from inside the .jar) is really affected if the resource already exists in cljminecraft. I'd hope it's the case, but either way I'd want to know

[CmdrDats]

Ye, I'd imagine that if a resource exists in cljminecraft, it would always shadow an existing resource in a different plugin since I believe it uses the classloader to find the resource :)

If you can get it to work without the class-loader-of, I'd be very interested to see your solution - I'm a bit wary with automatically including the jar in the classpath for when you mix it with non-clojure plugins - you wouldn't want to include other plugins too, so just need to keep that in mind - that's why the class-loader-of is a very nicely contained and explicit solution.

Thanks again for the time you're putting into this, I appreciate it.

[ghost]

Yeah I think I just did it, but I still have to do some checks and see what actually happens in the case when I set a thread classloader without restoring it, but so far I am pleased to how it is...

One thing I notice is that the classpath only shows one .jar at any one time, but with class-loader-of is used to show both cljminecraft and memorystone, but I figure it only showed one jar because the parent classloaders each had others .jars like the bukkit jar or who knows what... guessing

EDIT: well, I got nothing better to do and I kinda enjoy it :D

[CmdrDats]

Another thing to check is if you have two children plugins, they should be able to call each other's functions :) Or at least the one child plugin should be able to call the other - since you shouldn't get into a state of having circular jar dependencies anyhow.

[ghost]

I'll postpone that check, so far I only did call from memorystone some cljminecraft method called info (which is actually in the java hmm) I think i need some sleep though :D I'll get back on it after

[ghost]

actually I have to test it now :D then sleep I already made a copy of memorystone and named it moomoo but how do you suggest to do the using of say memorystone methods ? via require in the ns declaration? or at runtime somehow ?

[CmdrDats]

hehe - via using the ns declaration as you would any other clojure library :)

[ghost]

do I need to include memorystone into leiningen deps ? else it failed :D

[CmdrDats]

Ye, you should include memorystone in your lein deps :)

On Tue, Dec 4, 2012 at 12:59 PM, basicsensei notifications@github.comwrote:

do I need to include memorystone into leiningen deps ? else it failed :D

— Reply to this email directly or view it on GitHubhttps://github.com/CmdrDats/clj-minecraft/issues/4#issuecomment-10992376.

[ghost]

and in plugin.yml ? lol I can't think atnm

[ghost]

I suspected it would fail lol cause it causes clojure code to try to load using the same RT.loadResourceScript which requires that evil thread context class loader to be set, let' seee

[ghost]

I got a few of these

12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namesp
ace: memorystone.core, being replaced by: #'memorystone.core/test

12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namesp
ace: memorystone.other, being replaced by: #'memorystone.other/test

12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namesp
ace: memorystone.other2, being replaced by: #'memorystone.other2/test

maybe I shouldn't use require?

EDIT: but anyway, it works if I do this once Thread.currentThread().setContextClassLoader( getOurClassLoader() ); and never restore it :D EDIT2: but I'll check back later EDIT3: thanks a lot for your feedback and support, much appreciated.

[CmdrDats]

Just call your 'test' method something other than test :D

On Tue, Dec 4, 2012 at 1:10 PM, basicsensei notifications@github.comwrote:

I got a few of these

12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namespace: memorystone.core, being replaced by: #'memorystone.core/test 12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namespace: memorystone.other, being replaced by: #'memorystone.other/test 12:07:54 [SEVERE] WARNING: test already refers to: #'clojure.core/test in namespace: memorystone.other2, being replaced by: #'memorystone.other2/test

maybe I shouldn't use require?

— Reply to this email directly or view it on GitHubhttps://github.com/CmdrDats/clj-minecraft/issues/4#issuecomment-10992675.

[ghost]

oh i got it, it's because it's defined as test, not due to the require importing cool!

[ghost]

ok it works as it is committed currently on my fork (but I must recheck and consider what's actually happening in terms of evil side effects) I've the moomoo plugin (which is only locally), has

(ns moomoo.core
  (:require [cljminecraft.events :as ev])
  (:require [cljminecraft.player :as pl])
  (:require [cljminecraft.bukkit :as bk])
  (:require [cljminecraft.logging :as log])
  (:require [cljminecraft.files :as files])
  (:require [memorystone.core :as mscore])
  (:require [memorystone.other :as msother])
  (:require [memorystone.other2 :as msother2])
  )

....

(defn start
  [pluginInstance]
  (. pluginInstance info "in start moomoo.")
  (log/info "%s" "in start moomoo")
  (start2 pluginInstance)
  (mscore/test2 pluginInstance "a" 1 "c")
  (msother/test2 pluginInstance "a" 2 "c")
  )

(defn stop
  [pluginInstance]
  (. pluginInstance info "in stop moomoo.")
  (log/info "%s" "in stop moomoo")
  (msother2/test2 pluginInstance "a" 3 "c")
  ;TODO: deregister events?
  )

and project.clj dependencies [memorystone "2.0.0-SNAPSHOT"] and plugin.yml depend: [cljminecraft, memorystone]

peace out, for now

[ghost]

I discovered some things... not even sure where to start :))

but the main thing is this, I've managed to duplicate the behavior of class-loader-of but without actually using it, and without the side-effect of having config.yml shadowed but yet both are still having the same classpath which is including all the .jar files from the clojure plugins (that is, in my case, 3: cljminecraft, memorystone, moomoo -the test plugin)

so what did I do? well what I didn't do actually is set thread class loader - but instead I only bind the dynamic var *loader* aka clojure.lang.Compiler.LOADER to the commonly shared classloader (between all clojure plugins including cljminecraft) and this classloader is (as I said the same instance for all plugins) newCL = new clojure.lang.DynamicClassLoader( this.getClass().getClassLoader() ); which happens only once in the static initializer block. btw this.getClass().getClassLoader() resolves to something like org.bukkit.plugin.java.PluginClassLoader@5e6458a6 and it has this classpath { file:/S:/cb/plugins/clj-minecraft-1.0.1-SNAPSHOT-standalone.jar } , this is pretty fixed

to bind that *loader* I actually do this only once(in the thread) clojure.lang.Var.pushThreadBindings( clojure.lang.RT.map( clojure.lang.Compiler.LOADER, newCL) ); without ever calling popThreadBindings

now the tricky part is that whenever we first time call any RT "things" the RT class needs to be initialized(on load?) and this actually wants to load clojure/core so to work around this, I have to surround the above *loader* call with a setting the thread class loader (to ClojurePlugin.class.getClassLoader()) and then restoring it. This only happens once (not once for every clojure plugin), literally should be only once in bukkit lifetime because it's on static initializer block of ClojurePlugin(well in BasePlugin class really on my fork) class of cljminecraft.

... not sure what else to say at this point, EDIT: I'm in the process of cleaning up the code thoughts?

[ghost]

actually there is no clojure counterpart for clojure.lang.Compiler.LOADER other than just clojure.lang.Compiler/LOADER

[CmdrDats]

Cool, that's awesome - I had taken a similar route to this before implementing the class-loader-of, but nothing this thorough - if you clean up the code and make a pull request, I'll happily pull it in :)

[ghost]

oh man, there are so many other changes, I don't even know how to make a pull request to include only this change :) I could make a sample pull request just so you see the evil diffs EDIT: i mean, like there's a lot of garbage which I would like to keep actually (on my fork anyway) even on memorystone EDIT2: I made pull requests for both projects, just look at the diffs and maybe cherry pick or something :D (I already removed the tests which moomoo -test plugin - used)

[CmdrDats]

Hehe, I just pulled in everything - I reckon we can clean up as we go along, less pain than trying to manage individual commits or cherry picking, especially for future pull requests.

On the subject of 'garbage' you'd like to keep on your fork - maybe give it a little thought as to why it's valuable for you? Perhaps there's a way it can be transformed from garbage to something more permanent and useful to others?

[ghost]

I reckon we can clean up as we go along...

yeah sounds good to me.

About that garbage thing, I find it valuable somehow, at least in knowing that I've tried that variant (just in case to not try it again, or in case I'm wondering about that see that I've tried it and there's the answer - usually in comments). There may be redundant comments in more than 1 places which I consider garbage, or even some non-redundant comments. Maybe even some asserts - even though I consider them as stating the obvious (so that there's like no doubt/wondering time wasted). But also some commented-out code which seems like garbage but tell me that I've tried this or that or reminds me of some things... Unfortunately some comments may become outdated and linger on - that's kinda sad but that's what happens when we don't have a nice system in which to define things that depend on other things and at least get a warn if not a constraint when changing stuff... My failing is that I'm trying to cover as many cases as possible but due to limitations between "systems" (so to speak) it's not always possible to do and that's kinda unacceptable to me :dash: I just think about reify-ing some existing methods of a running plugin then having to disable our plugin ... I don't know there seem to be countless things that I'd want to do but we cannot really do within this system ... What was I talking about? peace out :)