-
Notifications
You must be signed in to change notification settings - Fork 27
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
Change how the default class translation files path is constructed #106
base: main
Are you sure you want to change the base?
Change how the default class translation files path is constructed #106
Conversation
042cb46
to
753db13
Compare
Hi James, just a reminder about this one in case it got lost 🙈 |
My apologies @marko-bekhta. I'll try to look at it this week :) |
I've filed https://issues.redhat.com/browse/LOGTOOL-158 for this. I will have a look at it tomorrow. I want to think about this. I need to refresh my memory a bit too :) |
FileObject fObj = processingEnv.getFiler().getResource(StandardLocation.CLASS_OUTPUT, packageName, interfaceName); | ||
classTranslationFilesPath = fObj.toUri().getPath().replace(interfaceName, ""); | ||
// Create some random name: | ||
String relativeName = interfaceName + UUID.randomUUID(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's weird to me is the exception is java.io.FileNotFoundException
, yet using a random file name makes it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, in the original approach, it was trying to get a resource (and eclipse compiler wants resources to actually exist) while this patch is just creating a new resource (so a potential problem could be that a file with that "random" name actually exists 🙈)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, sorry getResource()
vs createResource()
. I should have noticed that.
Hey James 😃 mvnDebug clean install -Pcompiler-eclipse and attaching a remote debugger in an IDE.
And I didn't want to dig into We can drop this change once we are happy with the fix or however you'd like to proceed. |
I've gotta be honest here, this seems like an awfully big change for a bug in a different project. I'm not sure I'd really want to put this fix in here when the fix should really be elsehwere. |
Just to be clear 🙈 only the first commit may be of use here (this part https://github.com/jboss-logging/jboss-logging-tools/pull/106/files#diff-aac55a253f897279072fe694e8d4b12f4444838b5d8c121257facd3382320fa1R154-R161). The second one is here temporarily, just in case you want to debug the behaviour of the Eclipse compiler. I agree that the error I've mentioned in the comment above (#106 (comment)) should be addressed either in |
Hey @jamezp
with the most recent release of
[plexus-compiler](https://github.com/codehaus-plexus/plexus-compiler)
and in particular after this fix: codehaus-plexus/plexus-compiler#350 we've started to see build failures with:e.g. https://ci.hibernate.org/blue/organizations/jenkins/hibernate-search/detail/main/1041/pipeline/121
Let me know if the change makes sense to you. Thanks!