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

Android Studio Integration: Attempt via bsp for Java android apps #4362

Merged

Conversation

vaslabs
Copy link
Contributor

@vaslabs vaslabs commented Jan 17, 2025

This PR provides

  • ability to import a mill android project to android studio via BSP, including sources and generated sources.
  • necessary changes to the way sources are resolved (through module dependency instead of class inheritance).
  • Changes to tests (e.g. number of sources compiled) reflect these changes

Not provided

  • Compilation still needs to be done from the command line in order for the paths to be resolved inside the IDE.

Note that to make compilation, running, testing and debugging work from android studio, we'll need to develop a plugin for it

Note

This is an early draft and I need to cleanup or review a few rough edges, I submit this as a draft to get some early feedback.

I've implemented java to get the basics to work, as kotlin has multiple kinds of sources and files that need to be taken care of, so this is just a small step.

Don't hesitate to point out any issues as I've really just braced through and hacked around to get this to work!

@0xnm @lihaoyi any feedback will be appreciated!

Android Studio Integration

The attempt focuses on BSP. Although bsp as a plugin does not seem to be available for android studio, it strangely becomes available if it is installed through intelliJ (see video)

Basic changes

  • Use the moduleDeps instead of resolving the classpath bits (sources, resources) through inheritance. This makes BSP and studio integration work (see the directories that are highlighted correctly)
  • Crude implementation of bsp methods to get the IDE to report the imported modules correctly

Tricky parts

  • Resources are compiled twice due to how aapt linking and later the module deps compile hierarchy. In order to avoid this, I've added an empty generatedSources method in android instrumentation module .

Demo

You can see in the demo most of the IDE static import features work (R is not recognised but I'll look into it either in this PR or subsequent).

Screencast.From.2025-01-17.18-41-15.mp4

EDIT: I've fixed the R not being recognised and the kotlin import

image

EDIT 2: Added summary at the top

@vaslabs vaslabs force-pushed the android/android-studio-experiments branch 2 times, most recently from e6f78de to 8ac7b18 Compare January 19, 2025 08:57
@vaslabs
Copy link
Contributor Author

vaslabs commented Jan 19, 2025

Saw this and I'm wondering if I should also give it a go with a maven import and maybe test the conversion of android apps to mill

@lihaoyi
Copy link
Member

lihaoyi commented Jan 19, 2025

Saw this and I'm wondering if I should also give it a go with a maven import and maybe test the conversion of android apps to mill

Give it a try! I expect it will not work out of the box, but maybe it can be made to work with some effort

@@ -1030,11 +1076,15 @@ trait AndroidAppModule extends JavaModule {

override def androidEmulatorPort: String = parent.androidEmulatorPort

override def sources: T[Seq[PathRef]] = parent.sources() :+ PathRef(androidTestPath / "java")
override def sources: T[Seq[PathRef]] = Seq(PathRef(androidTestPath / "java"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good change, but still, the way instrumented testing is implemented right now is different from the way AGP does it.

The current approach is to generate a single fat APK, consisting of production + test files.

AGP is doing it in a different way: it will be 2 APKs, one is a normal production APK, another one will contain only test files and will also have a dedicated manifest obviously, with a dedicated package name attribute which will be ${main app package name}.test. And this test APK should be installed with -t option (see here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right, I have decompiled the APKs from AGP as well and can work towards this if we want to have the exact same behaviour (which makes sense to have)

image

I want to set up a scaffold with some tests and examples working first and then do the underlying implementations on the next stage.

@vaslabs vaslabs force-pushed the android/android-studio-experiments branch from 8ac7b18 to cd85eea Compare January 20, 2025 09:55
Copy link
Contributor

autofix-ci bot commented Jan 20, 2025

Hi! I'm autofix logoautofix.ci, a bot that automatically fixes trivial issues such as code formatting in pull requests.

I would like to apply some automated changes to this pull request, but it looks like I don't have the necessary permissions to do so. To get this pull request into a mergeable state, please do one of the following two things:

  1. Allow edits by maintainers for your pull request, and then re-trigger CI (for example by pushing a new commit).
  2. Manually fix the issues identified for your pull request (see the GitHub Actions output for details on what I would like to change).

@vaslabs
Copy link
Contributor Author

vaslabs commented Jan 20, 2025

Saw this and I'm wondering if I should also give it a go with a maven import and maybe test the conversion of android apps to mill

Give it a try! I expect it will not work out of the box, but maybe it can be made to work with some effort

will do after I give a closer look into it, this PR should be ready soon (once everything passes)

@vaslabs vaslabs force-pushed the android/android-studio-experiments branch 3 times, most recently from 71e3ba4 to 51469bd Compare January 20, 2025 13:57
@vaslabs
Copy link
Contributor Author

vaslabs commented Jan 20, 2025

getting a failure on this job, so I haven't marked it as ready yet, but I don't think it's related to my changes

@lihaoyi
Copy link
Member

lihaoyi commented Jan 21, 2025

@vaslabs might be flakiness, feel free to ignore for now

Cleanup

Progressing on android studio integration

Progressing slowly

remove

Imports and tests working but with a hack

semi decent solution
@vaslabs vaslabs force-pushed the android/android-studio-experiments branch from 7cd9cc8 to 034b3ed Compare January 21, 2025 07:51
@vaslabs vaslabs marked this pull request as ready for review January 21, 2025 08:46
@vaslabs
Copy link
Contributor Author

vaslabs commented Jan 21, 2025

I'm done with the changes within the scope of this PR

@lihaoyi
Copy link
Member

lihaoyi commented Jan 21, 2025

Thanks @vaslabs and @0xnm for the review, looks good to me

@lihaoyi lihaoyi merged commit 7943110 into com-lihaoyi:main Jan 21, 2025
40 checks passed
@lefou lefou added this to the 0.12.6 milestone Jan 21, 2025
gamlerhart pushed a commit to gamlerhart/mill that referenced this pull request Feb 9, 2025
…m-lihaoyi#4362)

## This PR provides

- ability to import a mill android project to android studio via BSP,
including sources and generated sources.
- necessary changes to the way sources are resolved (through module
dependency instead of class inheritance).
- Changes to tests (e.g. number of sources compiled) reflect these
changes

### Not provided

- Compilation still needs to be done from the command line in order for
the paths to be resolved inside the IDE.

Note that to make compilation, running, testing and debugging work from
android studio, we'll need to develop a plugin for it

## Note

This is an early draft and I need to cleanup or review a few rough
edges, I submit this as a draft to get some early feedback.

I've implemented java to get the basics to work, as kotlin has multiple
kinds of sources and files that need to be taken care of, so this is
just a small step.

Don't hesitate to point out any issues as I've really just braced
through and hacked around to get this to work!

@0xnm @lihaoyi  any feedback will be appreciated!

## Android Studio Integration

The attempt focuses on BSP. Although bsp as a plugin does not seem to be
available for android studio, it strangely becomes available if it is
installed through intelliJ (see video)


### Basic changes

- Use the moduleDeps instead of resolving the classpath bits (sources,
resources) through inheritance. This makes BSP and studio integration
work (see the directories that are highlighted correctly)
- Crude implementation of bsp methods to get the IDE to report the
imported modules correctly

### Tricky parts

- Resources are compiled twice due to how aapt linking and later the
module deps compile hierarchy. In order to avoid this, I've added an
empty generatedSources method in android instrumentation module .

### Demo

You can see in the demo most of the IDE static import features work (R
is not recognised but I'll look into it either in this PR or
subsequent).


https://github.com/user-attachments/assets/77fe5e52-57e7-4480-9971-b50cc8d4d839



EDIT: I've fixed the R not being recognised and the kotlin import


![image](https://github.com/user-attachments/assets/69d810c9-9dd5-40ca-98d3-387d8693cc0e)


EDIT 2: Added summary at the top
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.

4 participants