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

Prepare Jdeps extension for K2 implementation #1164

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

jbarr21
Copy link
Contributor

@jbarr21 jbarr21 commented Apr 26, 2024

This PR makes 3 small changes to the JDeps extension in preparation of adding the K2 impl (#843):

  1. Ensure that we set the correct argument to enable K2 in KotlinJvmTestBuilder.java. We also avoid resetting the option by modifying the existing infoBuilder instead of using CompilationTaskInfo.newBuilder().
  2. Since the K1 & K2 flows will be different, they will track classes in a different order. We need to sort the deps in the proto so that we can properly assert their equality.
  3. We split out the Jdeps creation logic from JdepsGenExtension into a new base class that can be extended by the K2 implementation. There are no logic changes in this refactor.

Copy link
Collaborator

@Bencodes Bencodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you take a look at the KtLint errors?

@jbarr21
Copy link
Contributor Author

jbarr21 commented Apr 26, 2024

Can you take a look at the KtLint errors?

@Bencodes already fixed. the test failure passes locally though. any thoughts?

❯ bazel test //src/test/kotlin/io/bazel/worker:WorkerEnvironmentTest
...
INFO: Found 1 test target...
Target //src/test/kotlin/io/bazel/worker:WorkerEnvironmentTest up-to-date:
  bazel-bin/src/test/kotlin/io/bazel/worker/WorkerEnvironmentTest.jar
  bazel-bin/src/test/kotlin/io/bazel/worker/WorkerEnvironmentTest.jdeps
INFO: Elapsed time: 24.900s, Critical Path: 16.03s
INFO: 365 processes: 17 internal, 312 darwin-sandbox, 36 worker.
INFO: Build completed successfully, 365 total actions
//src/test/kotlin/io/bazel/worker:WorkerEnvironmentTest                  PASSED in 0.4s

@Bencodes
Copy link
Collaborator

Ah this one is flaky. Should be good to merge.

) :
AnalysisHandlerExtension, StorageComponentContainerContributor {
configuration: CompilerConfiguration,
) : BaseJdepsGenExtension(configuration),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this needs to be a base class and not just some independent class that both implementations can call into?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either one works, but I figured I was going to use a common base for the 2 extensions anyways

@Bencodes Bencodes merged commit 9a8c568 into bazelbuild:master Apr 26, 2024
1 of 2 checks passed
@jbarr21 jbarr21 deleted the jb/jdeps-k2-prep branch April 26, 2024 19:52
jbarr21 added a commit to uber-common/rules_kotlin that referenced this pull request May 1, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
restingbull pushed a commit that referenced this pull request May 31, 2024
* Fix K2 jdeps test args

* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
jbarr21 added a commit to uber-common/rules_kotlin that referenced this pull request Jul 3, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
jbarr21 added a commit to uber-common/rules_kotlin that referenced this pull request Jul 22, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Jul 31, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Jul 31, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 2, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 2, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 2, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 4, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 4, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Aug 4, 2024
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
Bencodes pushed a commit to Bencodes/rules_kotlin that referenced this pull request Aug 19, 2024
* Fix K2 jdeps test args

* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Jan 22, 2025
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Jan 22, 2025
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
oliviernotteghem pushed a commit to uber-common/rules_kotlin that referenced this pull request Jan 22, 2025
* Fix K2 jdeps test args
* Sort Deps in jdeps

* Refactor jdeps extension to prepare for K2 impl
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.

2 participants