Status Update
Comments
li...@gmail.com <li...@gmail.com> #2
As I understand it, we need to migrate namespace definitions both from AndroidManifest.xml
and from implicit definitions from applicationId
/ testApplicationId
? Is that right? So an 8.0 project would have a
namespace <n>
testNamespace <tn>
where <n> is taken from AndroidManifest.xml
package
definitions if present, and applicationId
if not, and <tn>
is testApplicationId
if present and <n>.test
if not?
ch...@google.com <ch...@google.com>
li...@gmail.com <li...@gmail.com> #3
Sorry for the confusion. AGP would change how it computes the namespace for the androidTest variant. All the Upgrade assistant would need to do is update the source code for the import of the R
class.
je...@google.com <je...@google.com>
sp...@google.com <sp...@google.com> #4
OK! And, I think, this should happen at the same time as the namespace
declaration is migrated from AndroidManifest.xml
?
ra...@google.com <ra...@google.com>
pa...@google.com <pa...@google.com>
pa...@google.com <pa...@google.com> #5
It's unrelated, but it is possible that we move 8.0 to also only declare the namespace into the DSL. I would make it a separate refactoring because they are not tied at all.
li...@gmail.com <li...@gmail.com> #6
I would consider first adding the right new value to models (as a new property) and then, to avoid Studio interpreting manifests and build configurations, just update to whatever the synced value of this new property is.
pa...@google.com <pa...@google.com> #7
Updated version of this in #16 below
Currently, AUA removes the package
attribute from the main manifest and instead specifies that value via the namespace
DSL.
In addition to that, AUA should:
(1) Check for apackage
attribute in the (possibly non-existent) androidTestAndroidManifest.xml
. If there's a value there, it should be removed from that manifest, and if it's not equal tonamespace + ".test"
, it should be specified via thetestNamespace
DSL.Ifnamespace
==testNamespace
, this is problematic because the androidTest and regularR
classes will have the same namespace. Not sure what the best behavior is here? Maybe an error explaining that the user should first reset the androidTest AndroidManifest'spackage
attribute and change the androidTest source code package accordingly?
- (2)
Compare the newtestNamespace
with the oldAndroidProject.androidTestNamespace
from the model. If they are different, search the androidTest source code for references to anR
class namespaced withAndroidProejct.androidTestNamespace
, and change the namespace totestNamespace
.
li...@gmail.com <li...@gmail.com> #9
I have a draft implementation for the second part of (1) from namespace
and testNamespace
are equal. Is there any documentation we can point to explaining how the user should proceed? (My in-window space budget for explanatory text is pretty limited).
sa...@google.com <sa...@google.com> #10
+Amy
Amy, would it make sense to add documentation about setting the namespace for testing? Similar to what we have
It could mention the following:
- the default test namespace is
namespace
+".test"
- to customize the test namespace, use the
DSLtestNamespace - If setting a value for
testNamespace
, users should not set the same value asnamespace
, otherwise the test and testedR
andBuildConfig
classes will have the same namespace.
sa...@google.com <sa...@google.com> #11
Yeah definitely, I can draft it and send you a CL. Thanks! Looks like we might need to update this section too: package
attribute is only used to set the applicationId
when you first create your project, right (assuming people are setting the namespace using the DSL)?
Would it also be helpful to add a release note about this? I guess it's technically not new with Chipmunk, but looks like we didn't mention it in the last couple stable releases.
li...@gmail.com <li...@gmail.com> #12
Looks like we might need to update this section too:
https://developer.android.com/guide/topics/manifest/manifest-element#package
Yes, setting the package attribute in the manifest is deprecated, will cause build warnings in AGP 7.3.0, and will cause build errors in AGP 8.0.
Currently the package attribute is only used to set the applicationId when you first create your project, right (assuming people are setting the namespace using the DSL)?
The namespace
DSL will always trump the package
attribute, even when determining the applicationId
. E.g., if a dev sets package
to "com.example.package", namespace
to "com.example.namespace", and doesn't set the applicationId
explicitly in the DSL, the applicationId
will be "com.example.namespace".
Would it also be helpful to add a release note about this? I guess it's technically not new with Chipmunk, but looks like we didn't mention it in the last couple stable releases.
I think a 7.3 release note and an 8.0 release note since we're warning starting in 7.3 and will break the build in 8.0 when the manifest has a package attribute.
pa...@google.com <pa...@google.com>
pa...@google.com <pa...@google.com> #13
Gotcha, thanks--sent you cl/449846661, to start. Is the warning/removal happening in certain Dolphin/EE preview releases, or not until the stable releases?
li...@gmail.com <li...@gmail.com> #14
li...@gmail.com <li...@gmail.com> #15
Okay sounds good, I added the Dolphin preview release note to cl/450515477 (messed up the previous CL) as well. Please let me know if you need help whenever the EE canary with the update is released!
Description
DESCRIBE THE ISSUE IN DETAIL:
STEPS TO REPRODUCE:
./gradlew :app:lintDebug
and observe 2 issues;./gradlew :lint-checks:test
and observe tests failing.It looks like
PartialResult#map
doesn't follow the contract in unit tests. It claims to returnLintMap
for the project in context, but under the certain condition it seems to be violated.The conditions require multimodule setup. A
library
module must put smth into itsLintMap
. Then, when theapp
module accesses its ownLintMap
for the first time it get a map prefilled with the data from thelibrary
module, instead of the fresh one.To demonstrate the issue I created the following setup:
:app
module and a:library
module;:app
module containsapp_color
color resource, while:library
module contains:library_color
;color
resource declaration within the module, puts into the partial results and then reports all colors found per module incheckPartialResults
method;If you run lint checks using the
./gradlew :app:lintDebug
command, you'll observe Lint correctly reportingapp_color
found in:app
andlibrary_color
found in:library
. However, if you run the unit tests, you'll observe that Lint also reports alibrary_color
found in the:app
.It looks like while scanning the
:app
module, the first access to partial results, causesLintCliClient
to create new instance of results and fill it with results of the dependent module (seeLintCliClient#getPartialResults
method). For some reason this is happening only in tests.The replacement of the
getPartialResult.map()
method call withgetPartialResult.mapFor(context.project)
method call successfully addresses the issue.There's still a chance, that my test is wrong. In this case, would be great to know in which way.
Studio Build: Chipmunk | 2021.2.1 Version of Gradle Plugin: 7.2.1, 7.4.0-alpha08 Version of Gradle: 7.4.2 OS: Mac OS Monteray 12.4