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

Clean up dependency mess #101

Closed
yawkat opened this issue Jan 3, 2025 · 3 comments · Fixed by #104
Closed

Clean up dependency mess #101

yawkat opened this issue Jan 3, 2025 · 3 comments · Fixed by #104
Assignees
Labels
type: improvement A minor improvement to an existing feature

Comments

@yawkat
Copy link
Member

yawkat commented Jan 3, 2025

Issue description

The dependency layout of this project is a bit messy at the moment. fuzzing-tests and fuzzing-annotation-processor are normal modules. jazzer-plugin is a subproject that is used as a plugin in fuzzing-tests. However, jazzer-plugin reads metadata generated by fuzzing-annotation-processor, so it needs a dependency on the model class, which right now is realized through a symlink.

This is a very bad solution and I'd prefer an actual dependency on fuzzing-annotation-processor. That doesn't work with the fact that jazzer-plugin is a subproject, though. Maybe it's better to make jazzer-plugin a normal module, and fuzzing-tests a subproject somehow?

I'm also open to moving fuzzing-tests to an entirely separate github project, if necessary. But I'd prefer to keep it here.

@yawkat yawkat added the type: improvement A minor improvement to an existing feature label Jan 3, 2025
@melix
Copy link
Contributor

melix commented Jan 7, 2025

So I've been contemplating this and trying to figure out a solution, but that's indeed quite a mess. It seems to me that the easiest is simply to not reuse the model class, but add a different one. It's simply used for JSON mapping, so it feels a bit overkill to reuse an artifact for this.

@yawkat
Copy link
Member Author

yawkat commented Jan 7, 2025

Okay, fair enough. The symlink is not a big issue either, but I could replace it with a simple copy of the model when the model stabilizes.

melix added a commit that referenced this issue Jan 7, 2025
This commit reworks how the model class is "shared" between the annotation
processor and the jazzer plugin. Instead of using a symlink, the code is
now generated by a common build plugin. In practice, the 2 modules use the
same record, but in a different package. It's also cleaner in the sense
that there's no dependency between the plugin and the annotation processor,
which would have led to too many classes on classpath.

Fixes #101
@melix
Copy link
Contributor

melix commented Jan 7, 2025

I have created a PR which I think makes things a bit nicer.

melix added a commit that referenced this issue Jan 7, 2025
This commit reworks how the model class is "shared" between the annotation
processor and the jazzer plugin. Instead of using a symlink, the code is
now generated by a common build plugin. In practice, the 2 modules use the
same record, but in a different package. It's also cleaner in the sense
that there's no dependency between the plugin and the annotation processor,
which would have led to too many classes on classpath.

Fixes #101
yawkat pushed a commit that referenced this issue Jan 7, 2025
This commit reworks how the model class is "shared" between the annotation
processor and the jazzer plugin. Instead of using a symlink, the code is
now generated by a common build plugin. In practice, the 2 modules use the
same record, but in a different package. It's also cleaner in the sense
that there's no dependency between the plugin and the annotation processor,
which would have led to too many classes on classpath.

Fixes #101
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants