GlowstoneMC / Glowstone-Legacy

An open-source server for the Bukkit Minecraft modding interface
Other
363 stars 122 forks source link

Library Management #143

Open zml2008 opened 10 years ago

zml2008 commented 10 years ago

Quick proposal, any thoughts?

I'd like libraries not to be shaded directly in the jars. Instead, dependencies could be specified in the glowstone.yml like so:

libraries:
- 'org.xerial:sqlite-jdbc:3.7.2'
- 'mysql:mysql-connector-java:5.1.14' # maybe optional version specifiers

These files would be verfied and downloaded if necessary (maybe even from maven central since that already exists, verifying md5+gpg if available) and added to the classpath available to plugins.

At some point this could allow plugins to define their own libraries to be available in the plugin.yml too.

lukespragg commented 10 years ago

Spot on @zml2008. One internet for you!

greatman commented 10 years ago

+1 internet to you sir.

ZephireNZ commented 10 years ago

Love the idea as well. Will definitely take us one step closer to using Glowstone as a drop-in replacement for CraftBukkit.

Although, what libraries should we download by default, if any? All the dependencies currently missing compared to CraftBukkit?

lukespragg commented 10 years ago

I'd go for Maven Central by default, Sonatype OSSRH as a backup, and allow them to specify others too in cause they have issues trying to get custom libs.

I wouldn't download any by default @ZephireNZ, instead detect if possible?

zml2008 commented 10 years ago

@ZephireNZ I'd include the database libraries by default

@lukespragg yeah, repo specifications would be good.

I'll look into making this happen a bit more.

zml2008 commented 10 years ago

This project looks like it has an implementation of maven dependency resolution we could use.

SpaceManiac commented 10 years ago

Yeah, I was interested in this. I think it'd be pretty nice, actually, to pull jars on first run rather than shade everything (so that dependencies that never change aren't redownloaded all the time).

It was mentioned on IRC that Forge (which does something similar) got in some shenanigans once due to causing load on Maven Central. Someone should ask them for more details on that so we don't repeat their mistake (if this actually happened).

Only the JDBC drivers are really necessary, I think. Anything else included in CraftBukkit but not Bukkit won't actually be used by pure Bukkit plugins.

JeromSar commented 10 years ago

I believe this is possible with Gradle.

dequis commented 10 years ago

Also finding some plugins failing due to org.apache.logging.log4j.core.Filter - this one would make sense to shade and use internally, IMO

zml2008 commented 10 years ago

Well that's a log4j issue. Vanilla MC uses log4j, bukkit doesn't, and afaik GS doesn't either. Plugins trying to work with log4j and not shading the necessary classes should be treated like any plugin that uses nms code.

On Thu, Sep 11, 2014 at 07:44:02PM -0700, dx wrote:

Also finding some plugins failing due to org.apache.logging.log4j.core.Filter - this one would make sense to shade and use internally, IMO


Reply to this email directly or view it on GitHub: https://github.com/GlowstoneMC/Glowstone/issues/143#issuecomment-55355481

Ribesg commented 10 years ago

@zml2008 got the same issue with my plugin suite, log4j Filters are used on CB to hide /login and /register command being logged in console/logs, showing plain passwords to the server admin.

I think we'll have to make some version check to either use Java loggers or Log4J loggers then :-/

EDIT: I mean, in our plugins, if we want to support both CB and Glowstone, we need to support both Java and Log4J filters.

dequis commented 10 years ago

I created a wiki page documenting how to workaround the lack of shaded libraries, using classpath:

https://github.com/GlowstoneMC/Glowstone/wiki/Library-Management

Any corrections would be appreciated. So far, seems to work with NCore+NWorld (log4j) and CoreProtect (sqlite).

Squawkers13 commented 10 years ago

JDBC should be downloaded by default.

Postremus commented 10 years ago

jdbc is integrated in java - the database drivers needs to be downloaded.

2014-09-30 15:38 GMT+02:00 Squawkers13 notifications@github.com:

JDBC should be downloaded by default.

— Reply to this email directly or view it on GitHub https://github.com/GlowstoneMC/Glowstone/issues/143#issuecomment-57314454 .

dequis commented 9 years ago

Wouldn't it be better to do library shading while this isn't implemented? That can be changed once a library manager lands in master, but meanwhile, there should be a way to have those plugins work out of the box. The classpath thing in the wiki is a workaround, and an ugly one.

I know that downloading them on runtime is the best solution, but the only one who worked on it is mastercoms (that WIP PR with commits from october 12 only, and only conflict fixes afterwards), and it's in a sorta abandoned state.

zml2008 commented 9 years ago

We'd have to run our own maven repo -- supposedly central has a majority of its traffic just from mc development, and they got really pissy when forge tried to download directly.

SpaceManiac commented 9 years ago

Based partially on the work of @mastercoms, the commit cb42c013c9ef02ccd003340099e3cf6092c95e74 (not yet in master) is a very simple first-run downloader for the sqlite and mysql connectors. It can later be expanded if more configuration is desired.

I don't think it makes sense to include the log4j libs since they wouldn't be hooked up to anything and aren't actually exposed by Bukkit (whereas MySQL and Sqlite are reasonably expected).

Ribesg commented 9 years ago

As I'm quite convinced plugins using Log4J are mainly using it for logging filters, I'm linking to what I did to my NPlugins suite to be able to filter the server logs in both cases (Log4J and Java Logging).

Everything's here. I didn't test it much but it's not complicated so it should work.

turt2live commented 9 years ago

I can say that the only time I've ever used log4j is to filter user passwords (warning: old code of mine, may be crap).

I would say that it's not needed though, as plugins who use it are generally only using it to modify logging, so if Glowstone doesn't use it then there's no point in including it unless we're going to use it.

Edit: Apparently I actually used the Java logging API... same justification though.

mastercoms commented 9 years ago

@SpaceManiac Sorry I didn't get around to doing it myself. Looks great though.

EDIT: should discuss how before doing dependency resolving.

SpaceManiac commented 9 years ago

The noted commit has landed in master as 5832722be1d5ea5d89fdc679a7db572015af1d2e, fixed to actually work.

I'm going to go ahead and not include log4j for now. If it's being used for filtering passwords it's better to fail loudly than silently, and for any other use (a plugin intended for logging filtering, etc.) some adaptation or fix to the plugin can be made to not rely on log4j.

This ticket remains open pending better configuration options and/or more sophisticated downloading.