Skip to content

Android: Standardize the use of application id and namespace #5210

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

Conversation

vaslabs
Copy link
Contributor

@vaslabs vaslabs commented May 27, 2025

This PR standardises the use of application id and namespace. It is also one of the pre-requisites of making hilt instrumented tests to work.

The application id is a unique id for every android app , so the android manifest package depends on it.

The android manifest package is also used whenever there's a relative path for Applications and activities so

<manifest package="com.foo">
    <application android:name=".TodoApplication"/>
</manifest>

Becomes

<manifest package="com.foo">
    <application android:name="com.foo.TodoApplication"/>
</manifest>

During manifest merging. Because this happens when the manifest is loaded (before everything is merged) we start with the manifest package being the android namespace. We then pass the package property to the merger to be the application id (the package for each up is unique). This is what appears to be happening in AGP as well. (It is also the reason why I was confused when reverse engineering the AGP behaviour and was alternating between namespace and application id)

I've also cleaned up some of the xml manual modifications that are not needed anymore as are handled by the merger tool

@vaslabs
Copy link
Contributor Author

vaslabs commented May 27, 2025

I think the job failures are not related to these changes

@vaslabs vaslabs marked this pull request as ready for review May 27, 2025 10:35
@vaslabs vaslabs force-pushed the android/sort-out-application-namespace-and-id branch from 83932dd to 646b429 Compare May 27, 2025 11:41
Copy link
Member

@lihaoyi lihaoyi left a comment

Choose a reason for hiding this comment

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

Looks good to me, happy to merge when you;re done with this

@vaslabs
Copy link
Contributor Author

vaslabs commented May 28, 2025

Looks good to me, happy to merge when you;re done with this

Yes, it's ready

@lihaoyi lihaoyi merged commit ce62087 into com-lihaoyi:main May 28, 2025
36 of 38 checks passed
@lefou lefou added this to the 1.0.0-RC2 milestone May 28, 2025
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.

3 participants