Glowstone-Legacy icon indicating copy to clipboard operation
Glowstone-Legacy copied to clipboard

Library Management

Open zml2008 opened this issue 9 years ago • 21 comments

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.

zml2008 avatar Sep 04 '14 03:09 zml2008

Spot on @zml2008. One internet for you!

lukespragg avatar Sep 04 '14 03:09 lukespragg

+1 internet to you sir.

greatman avatar Sep 04 '14 03:09 greatman

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?

ZephireNZ avatar Sep 04 '14 03:09 ZephireNZ

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?

lukespragg avatar Sep 04 '14 03:09 lukespragg

@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 avatar Sep 04 '14 03:09 zml2008

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

zml2008 avatar Sep 04 '14 04:09 zml2008

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.

SpaceManiac avatar Sep 04 '14 04:09 SpaceManiac

I believe this is possible with Gradle.

JeromSar avatar Sep 04 '14 18:09 JeromSar

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

dequis avatar Sep 12 '14 02:09 dequis

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

zml2008 avatar Sep 12 '14 03:09 zml2008

@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.

Ribesg avatar Sep 12 '14 08:09 Ribesg

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).

dequis avatar Sep 13 '14 05:09 dequis

JDBC should be downloaded by default.

Squawkers13 avatar Sep 30 '14 13:09 Squawkers13

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

2014-09-30 15:38 GMT+02:00 Squawkers13 [email protected]:

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 .

Postremus avatar Sep 30 '14 13:09 Postremus

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.

dequis avatar Nov 20 '14 18:11 dequis

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.

zml2008 avatar Nov 20 '14 18:11 zml2008

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).

SpaceManiac avatar Dec 23 '14 08:12 SpaceManiac

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.

Ribesg avatar Dec 23 '14 12:12 Ribesg

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.

turt2live avatar Dec 23 '14 14:12 turt2live

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

EDIT: should discuss how before doing dependency resolving.

mastercoms avatar Dec 23 '14 17:12 mastercoms

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.

SpaceManiac avatar Dec 24 '14 01:12 SpaceManiac