wix-incubator / exodus

Easily migrate your JVM code from Maven to Bazel
MIT License
222 stars 21 forks source link

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

Open natansil opened 4 years ago

natansil commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Sounds good. Were you able to confirm that in SBT zinc does show this cross module?

On Sun, 24 Nov 2019 at 11:15 Natan Silnitsky notifications@github.com wrote:

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 https://github.com/davidB/scala-maven-plugin/commit/ba7ab932530ce9a3090e4bbbcc778bc85f56c894#diff-04deabfe39517b4bc4d074ee47d1cc2dL76 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 https://github.com/ittaiz WDYT?

@D0rmouse https://github.com/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...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wix/exodus/issues/27?email_source=notifications&email_token=AAKQQF2P6ZMF7V4VFC4K7ODQVJA3ZA5CNFSM4JPGJ2JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFAG6ZY#issuecomment-557870951, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF6LVKBPYUM34Z6726LQVJA3ZANCNFSM4JPGJ2JA .

--

Ittai Zeidman

Cell: 054-6735021

40 Hanamal street, Tel Aviv, Israel

http://www.wix.com

natansil commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

stahloss commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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