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

FEATURE: Set config file via system property "lombok.configUri" #2643

Closed
wants to merge 1 commit into from

Conversation

mkurz
Copy link
Contributor

@mkurz mkurz commented Nov 9, 2020

I think the added changelog entry says it all:


  • FEATURE: You can now pass a config file via the system property lombok.configUri (e.g. when using Eclipse, passing -Dlombok.configUri=/path/to/lombok.config in eclipse.ini or, in general, setting it via the JAVA_TOOL_OPTIONS environment variable so javac picks it up). This might be useful if the used build tool uses some exotic path system. E.g. starting with version 1.4, sbt (https://www.scala-sbt.org/) uses vf: instead of file:, which lombok couldn't handle.

So as you can see sbt 1.4 introduces a virtual file system, so the "files" lombok "receives" from sbt are all prefixed with vf: (and also have a virtual path, which is different than the actual path on the file system), which lombok just ignores. However Lombok itself works fine, only the config is broken, meaning all lombok.config files are ignored (sbt/sbt#5968). The relevant code in Lombok can be found here (see the lengthy comment in this code snippet):
https://github.com/rzwitserloot/lombok/blob/737dc6e4333c09a0848d2f5b0d621a27a8039a70/src/core/lombok/core/configuration/FileSystemSourceCache.java#L67-L87

This fairly easy fix gives us a workaround, so we can just set the lombok.configUri env variable in build.sbt and the specified lombok.config gets picked up. I know with this environment variable we can only define just one config file instead of putting many different config files in different packages/folders, but IMHO having at least a "global" config file is good enough to set at least global config keys. Also, because I re-use the BubblingConfigurationResolver (mainly because it already handles imports inside a lombok config), bubbling up the directory tree still works.

@rspilker
Copy link
Collaborator

The problem with this approach is that it only seems to handle one config file, and not per package, and that the setting is per machine.

I think a better solution is to handle the "vf:" protocol correctly.

For a java source file, we need to be ably to get the contents of a lombok.config on the same "directory". And we need to walk up the tree, and resolve other paths from imports.

We would probably need a different implementation of the ConfigurationFile.

@rzwitserloot
Copy link
Collaborator

Sooo, big yikes on this reflective clownfest on commit e1f82ac - this is a bit fragile and I really would far rather use the AnnotationProcessor filer, but sbt's filer doesn't seem to get the job done, and whilst extremely ugly (spoiler if you haven't look at the commit yet: I use reflection to dig my way into a scala map and scala's Optional's Some to get what I need here. "yikes" indeed). But the good news is, it works, and it does what we want, which is: Find a config file that is sitting next to your source file, as well as apply any config files in any parent dirs, so that you can e.g. make a config file for a package, for a project, for a workspace, for your entire disk - etc.

Gracias for spotting this and giving it a try, @mkurz! For what it's worth, I'll leave the (dubious?) honour of commenting on the issue you filed with the sbt project. In fairness to them, we jump from the FileObject's URI to a file on disk and that really isn't a move that we can expect a compiler/build tool to support like this; the API doesn't make any suggestions that this conversion should just work, so that's on us, and therefore, it probably isn't fair to characterize this as a bug in sbt.

@mkurz
Copy link
Contributor Author

mkurz commented Nov 13, 2020

@rzwitserloot Thank you so much for taking the time and providing a fix! Can you do me a favour and update the edge release, so I can test the fix?
Also thank you for having a look @rspilker!

@mkurz
Copy link
Contributor Author

mkurz commented Nov 13, 2020

Also @rspilker

...and that the setting is per machine.

Not really. My intention with this pull request was that you can put something like this in the build.sbt

SettingKey[String]("set-lombok-config").withRank(KeyRanks.Invisible) :=
        "" + System.setProperty("lombok.configUri", baseDirectory.value.getAbsolutePath + "/lombok.config"),

so it would read lombok.config from the root of the project (which of course is also checked in into scm).
So no, this would not be per machine, but per project "only".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants