Skip to content

Added support for adding extra files to the APK. #141

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions cargo-apk/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

- Added support for adding extra files to the APK.

# 0.5.6 (2020-11-25)

- Use `dunce::simplified` when extracting the manifest's assets and resource folder
Expand Down
5 changes: 5 additions & 0 deletions cargo-apk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ res = "path/to/res_folder"
# If not specified, assets will not be included in the APK
assets = "path/to/assets_folder"

# Path to a folder containing a hierarchy of extra files to be added at the root of the APK.
# All files will be added to the APK recursively.
# If not specified, no extra files will be added to the APK
extra_content = "path/to/extra_content_folder"
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess assets/resources are intended to be relative paths included in version control, while in your use case it's probably an os specific path. Also this doesn't help select the right library for the target architecture. Are there additional use cases for extra-content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting that I write the example as an absolute path?
In my particular use-case I do have the shared library file under my repository directory (although not included in version control), so I'm using a relative path.

Regarding selection of the right library for the target architecture, if the APK contains multiple architecture, I think what happens is that dlopen() already appropriately uses the correct set of paths (including the proper lib subfolders in the APK) depending on which architecture is used when running the app.

As for additional use cases, honestly, I can't think of any off the top of my head.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we produce fat apk's because that can lead to very large apks. We could add the feature.

Copy link
Member

Choose a reason for hiding this comment

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

@dvc94ch Apparently cargo-apk already produces fat apks when more target architectures are specified in build_targets. I don't know if separate APKs can be distributed anyway unless we dive into the new "app bundles" system.

Regarding assets/resources we have to be very careful that those still travel the normal aapt package -A/-S as they need additional processing and are not "zipped" into the target APK verbatim.


# Adds application metadata to the manifest
# Note that there can be several application_metadatas entries
# this will add: <meta-data android:name="com.samsung.android.vr.application.mode" android:value="vr_only"/>
Expand Down
15 changes: 15 additions & 0 deletions cargo-apk/src/apk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ impl<'a> ApkBuilder<'a> {
)
.to_owned()
}),
extra_content: self.manifest.extra_content.as_ref().map(|res| {
dunce::simplified(
&self
.cmd
.manifest()
.parent()
.expect("invalid manifest path")
.join(&res),
)
.to_owned()
}),
};

let config = ApkConfig::from_config(config, self.manifest.metadata.clone());
Expand Down Expand Up @@ -125,6 +136,10 @@ impl<'a> ApkBuilder<'a> {
apk.add_lib_recursively(&artifact, *target, libs_search_paths.as_slice())?;
}

if let Some(content_dir) = &config.extra_content {
apk.add_content_dir_recursively(content_dir)?;
}

Ok(apk.align()?.sign(config.ndk.debug_key()?)?)
}

Expand Down
3 changes: 3 additions & 0 deletions cargo-apk/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub struct Manifest {
pub build_targets: Vec<Target>,
pub assets: Option<String>,
pub res: Option<String>,
pub extra_content: Option<String>,
}

impl Manifest {
Expand All @@ -28,6 +29,7 @@ impl Manifest {
build_targets: metadata.build_targets.unwrap_or_default(),
assets: metadata.assets,
res: metadata.res,
extra_content: metadata.extra_content,
})
}
}
Expand Down Expand Up @@ -55,4 +57,5 @@ struct AndroidMetadata {
build_targets: Option<Vec<Target>>,
assets: Option<String>,
res: Option<String>,
extra_content: Option<String>,
}
2 changes: 2 additions & 0 deletions ndk-build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

- Added support for adding extra files to the APK.

# 0.1.4 (2020-11-25)

- On Windows, fixed UNC path handling for resource folder
Expand Down
42 changes: 41 additions & 1 deletion ndk-build/src/apk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ use crate::error::NdkError;
use crate::manifest::Manifest;
use crate::ndk::{Key, Ndk};
use crate::target::Target;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::{
fs,
path::{Path, PathBuf},
};

pub struct ApkConfig {
pub ndk: Ndk,
pub build_dir: PathBuf,
pub assets: Option<PathBuf>,
pub res: Option<PathBuf>,
pub manifest: Manifest,
pub extra_content: Option<PathBuf>,
}

impl ApkConfig {
Expand Down Expand Up @@ -77,6 +81,7 @@ impl ApkConfig {
assets: config.assets,
res: config.res,
manifest,
extra_content: config.extra_content,
}
}

Expand Down Expand Up @@ -155,6 +160,41 @@ impl<'a> UnalignedApk<'a> {
Ok(())
}

pub fn add_file(&self, base_path: &Path, path: &Path) -> Result<(), NdkError> {
let out = self.0.build_dir.join(path);
let out_dir = out.parent().unwrap();
std::fs::create_dir_all(&out_dir)?;
std::fs::copy(base_path.join(path), out)?;

let mut aapt = self.0.build_tool(bin!("aapt"))?;
aapt.arg("add").arg(self.0.unaligned_apk()).arg(path);
Copy link
Member

Choose a reason for hiding this comment

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

Should we make sure that aapt runs relative to the build directory here? There are environment variables dealing with moved target/ directories and I don't exactly remember how they are handled here, if we keep running in the right PWD.

If we do that this might as well run aapt with base_path as PWD.

if !aapt.status()?.success() {
return Err(NdkError::CmdFailed(aapt));
}
Ok(())
}

fn add_entry(&self, base_path: &Path, path: &Path) -> Result<(), NdkError> {
let full_path = base_path.join(path);
if full_path.is_dir() {
for entry in fs::read_dir(full_path)? {
Copy link
Member

Choose a reason for hiding this comment

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

Can't we pass a directory to aapt add so that it adds them recursively?

If not you're better off building an array of filenames and adding them to the archive in one go, spawning an aapt add process per file can potentially become very costly and wasteful.

let entry = entry?;
let tgt_path = path.join(entry.file_name());
self.add_entry(base_path, &tgt_path)?;
}
} else if full_path.is_file() {
self.add_file(base_path, path)?;
}
Comment on lines +185 to +187
Copy link
Member

Choose a reason for hiding this comment

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

May need an error if it's not a directory nor a file (ie. a special file like a character device) just in case?

Ok(())
}

pub fn add_content_dir_recursively(&self, content_dir: &Path) -> Result<(), NdkError> {
if !content_dir.is_dir() {
return Err(NdkError::PathInvalid(content_dir.to_path_buf()));
Copy link
Member

Choose a reason for hiding this comment

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

I'd say something along the lines of "Is not a directory". That way the path can either not exist at all, or be a file. Invalid is a tad too generic IMO.

}
self.add_entry(&content_dir, Path::new(""))
}

pub fn align(self) -> Result<UnsignedApk<'a>, NdkError> {
let mut zipalign = self.0.build_tool(bin!("zipalign"))?;
zipalign
Expand Down
1 change: 1 addition & 0 deletions ndk-build/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct Config {
pub debuggable: bool,
pub assets: Option<PathBuf>,
pub res: Option<PathBuf>,
pub extra_content: Option<PathBuf>,
}

#[derive(Clone, Debug, Default, Deserialize)]
Expand Down
2 changes: 2 additions & 0 deletions ndk-build/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub enum NdkError {
},
#[error("Path `{0:?}` doesn't exist.")]
PathNotFound(PathBuf),
#[error("Path `{0:?}` is invalid.")]
PathInvalid(PathBuf),
#[error("Command `{0}` not found.")]
CmdNotFound(String),
#[error("Android SDK has no build tools.")]
Expand Down