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

Zinc Analysis does not provide source file dependencies from other maven modules #27

Open
natansil opened this issue Nov 19, 2019 · 14 comments

Comments

@natansil
Copy link
Collaborator

@D0rmouse, Unfortunately, with the help of your repo, I can indeed verify that maven's scala-plugin zinc-analysis does not provide source file dependencies from other maven modules.

What it does instead, is to provide the binary jar that the dependency comes from:
e.g. from your repo:

products/some-service/core-business-some-service/src/main/java/exodus/demo/core/business/some/service/impl/DefaultSomeService.java -> commons/core-common/target/core-common-0.0.1-SNAPSHOT.jar

I take full responsibility for missing this.
Two options I see:

  1. make Exodus emit uber-target for each maven module for other modules to depend on.
    This will mean more coarse-grain granularity for most of the dependencies.
  2. create a migration phase where the maven module structure is consolidated to 1 big module.
    I'm not sure it's feasible as this will mean that zinc compilation will work really hard and may run into limitations.

My intention for now is to try to work on 2. and see if it feasible.

If not then we can try option 1.

I apologise for this predicament.
Will keep you updated on my progress...

CC: @ittaiz

@stahloss
Copy link
Contributor

@natansil I'm glad the demo project helped in finding out what the issue is. And I'm glad to, not only because Bazel is awesome and anybody should be able to migrate away from the relic that is Maven with the least effort using your amazing migrator :)

Does it also account for the duplicate targets I've seen in that are generated in the package level BUILD.bazel files, or couldn't you replicate this using the demo project?

Wish you good luck in the effort trying to realize the best option!

@natansil
Copy link
Collaborator Author

I'll take a look if I see duplicates and get back to you.
I think a possible 3rd option for solving the cross-module dependencies issue, is to fork the scala-maven-plugin and pass zinc sources-jars for each module, than somehow let zinc use srcjar instead of binary jars. This will be the ultimate solution if it is feasible

@natansil
Copy link
Collaborator Author

Update: After going down multiple rabbit holes, I've focused on getting the relations output files to contain the class-name cross-module dependency (so that Exodus can use it in a 2-pass fashion to gather the source file cross-module dependencies). currently, it only contains the binary jar name. I've tried to look at the interaction between scala-maven-plugin and zinc.

One important note: scala-maven-plugin version 4.0.0 upgraded zinc dependency from 0.3.15 to 1.2.5 and with it the compile.relations files disappeared! (maybe this change caused it)

So the next AIs is to see how the relations file can be resurrected and then how to add the class-name dep from the binary jar (The information is present in Zinc - I'm almost positive).

@ittaiz WDYT?

@D0rmouse I plan on working on this during sporadic evenings and weekends, so I really can't promise quick resolution. I may be able to get some help from some colleagues but that would also take time.
If you want to check out the plugin code and see how to resurrect the relations file that would be great...

@ittaiz
Copy link
Contributor

ittaiz commented Nov 24, 2019 via email

@natansil
Copy link
Collaborator Author

I saw that in Maven, the cross module is shown, but as a binary jar, and also saw zinc code that is related to binary dependency information, so I'm not sure I need to check with SBT as well.

@stahloss
Copy link
Contributor

Update: After going down multiple rabbit holes

Remember what the dormouse said xD

@D0rmouse I plan on working on this during sporadic evenings and weekends, so I really can't promise quick resolution. I may be able to get some help from some colleagues but that would also take time.
If you want to check out the plugin code and see how to resurrect the relations file that would be great...

I've created several builds for our monorepo using commit filters. It's not ideal, but workable for now. I wasn't prepared to give up the single version advantage.
So there's no need for me to get this resolved quickly, but I'll see if I can help and keep you updated.

@natansil
Copy link
Collaborator Author

Can you please give a little more detail on your process and what are you trying to solve?

@stahloss
Copy link
Contributor

stahloss commented Nov 27, 2019

In our CI/CD environment (Azure DevOps) we're running several maven builds for our products in the monorepo by using commit filters.

For instance, if a changeset touches /products/some-product, a mvn clean install -pl /products/some-product -am is ran.
If a changeset touches commons/some-common-lib then all builds that have that path in their filters will be run.

Of course, preferably, you'd want your build tool to figure out what to build, like Bazel can, and then incrementally build and test. But that's not really possible with Maven.
You can build incrementally, but not test. And you'd only want artifacts rebuilt that are actually touched. Also not really possible with maven.

Since we cannot really migrate to Bazel quickly at the moment, I've taken the above approach for now.

@natansil
Copy link
Collaborator Author

I see. thanks for the detailed explanation!
I hope you will be able to move to bazel soon.
I'm also trying to get help from inside Wix.

@natansil
Copy link
Collaborator Author

Hi @D0rmouse.
I hope you are doing well.
I have some good news! we have solved the multi-module dependency issue.
Instead of relying on Zinc, we have switched to jdeps + javap for the analysis.
I would extremely appreciate if you could try it out (Exodus HEAD is already updated with this change)

@stahloss
Copy link
Contributor

stahloss commented Apr 28, 2020

Hi @natansil :)
Yes I'm fine thank you. How are you?
I'm not working from the company I attempted the migration for anymore, so this codebase is not available to me, but I made a demo project that can be found here. I don't have much time until next week at least, but you can try it with this setup yourself if you like.
Nice that you fixed it :)

@natansil
Copy link
Collaborator Author

Glad to hear you're ok :) I'm doing great. WFH of course.

I ran the new exodus version on the exodus-demo repo.
Looks promising.
inter-module dependencies are present in the build files!

There are spring related errors - but that is to be expected - some of these dependencies only appear in runtime. so analysis tools have a hard time with them.

also a small issue due to mix junit4/junit5 in the project

So Do you know anyone in the company where you tried this migration, that will be interested to continue your efforts?

How big is your current codebase :)

@stahloss
Copy link
Contributor

Sorry took a while...
That's great! Too bad I'm not able to continue with it.
At my old company (and at my new one as well) it's hard to find people that are monorepo enthusiasts :( People seem to have a hard time grasping the benefits far outweigh the drawbacks with it.
Of course, Bazel doesn't have to be used for monorepos only, but migrating makes less sense if you have numerous small projects.

@natansil
Copy link
Collaborator Author

We consolidated the repos to around 50. But as long as you have a shared remote repo cache, I'm not sure how worse the build performance would be.
The question is whether these have many dependencies between them...

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

No branches or pull requests

3 participants