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.

Sign up for GitHub

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

Add dexes to AAB #19

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Add dexes to AAB #19

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f68bd78
add dexes to aab
maxded Dec 6, 2024
c866383
docs
maxded Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions xbuild/src/gradle/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,36 @@ pub fn build(env: &BuildEnv, libraries: Vec<(Target, PathBuf)>, out: &Path) -> R
dependencies.push_str(&format!("implementation '{}'\n", dep));
}

let mut dexes = String::new();
for dex in &env.config().android().dexes {
let mut path = env.cargo().package_root().join(dex);

// Pop the filename and use the directory.
//
// This is needed as we must provide a directory to `DexMergingTask::dexDirs`
path.pop();
Comment on lines +74 to +77
Copy link
Member

Choose a reason for hiding this comment

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

Quite unfortunate: how do we control that there are no accidental/stale dex files in the directories? How did you deal with the "gradle does not know who produced this file, and cannot build a dependency graph" sort-of warning/error?

Copy link
Author

Choose a reason for hiding this comment

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

Quite unfortunate: how do we control that there are no accidental/stale dex files in the directories?

Right now, I don't. If there is a way to provide only files we should change to that but I haven't found it yet. If that doesn't exist, perhaps we should change the xbuild API to point to a directory in the manifest yaml instead of individual dex files.

How did you deal with the "gradle does not know who produced this file, and cannot build a dependency graph" sort-of warning/error?

By using a completely different task from yesterday. DexMergingTask not DexArchiveBuilderTask


let path = path.display().to_string().replace(r"\", r"/");

let external_lib = format!(r#"task.dexDirs.from("{path}")"#);
dexes.push_str(&external_lib);
dexes.push_str("\n");
}

let dexes = if !dexes.is_empty() {
format!(
r#"
afterEvaluate {{
tasks.named("mergeDexRelease").configure {{ task ->
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like this breaks if we build debug. As discussed and shown in-person yesterday, why not use tasks.withType(TheTypeThatMergeDex{}Uses)?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds like this breaks if we build debug.

Potentially but not sure yet. In evolve we're not building AAB in debug ever so I decided to stick with this solution for now. That being said, I do agree that it should also work on debug at some point!

As discussed and shown in-person yesterday, why not use tasks.withType(TheTypeThatMergeDex{}Uses)?

That is incorrect also as you could potentially add the directory to multiple tasks and you don't even really know which tasks. I prefer to be explicit and specify one task.

Copy link
Member

Choose a reason for hiding this comment

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

By that logic there could be multiple mergeDexRelease/Debug tasks in the chain, making this equally incorrect 🙃

Copy link
Author

Choose a reason for hiding this comment

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

By that logic there could be multiple mergeDexRelease/Debug tasks in the chain, making this equally incorrect 🙃

There are multiple dex merging tasks all with different names, most have "debug" and "release" somewhere in their task name making it quite confusing. I inspected all these dex merging tasks to see what input directories they process and where the task would place its output. Based on that info I made the decision to add our precompiled dex directory to mergeDexRelease. Is this objectively the best task to add it to, no clue.. it works and we should look at this again some other time.

{dexes}
}}
}}
"#
)
} else {
String::new()
};

let asset_packs = if config.assets.is_empty() {
""
} else {
Expand Down Expand Up @@ -94,6 +124,8 @@ pub fn build(env: &BuildEnv, libraries: Vec<(Target, PathBuf)>, out: &Path) -> R
dependencies {{
{dependencies}
}}

{dexes}
"#,
package = package,
target_sdk = target_sdk,
Expand Down
Loading