Support customized UDL file. #253
Conversation
Do not overwrite the existing UDL file, generate bindings based on UDL instead of copying. Overwrite with the default UDL only if no UDL is present in the project `src` directory.
|
(forward the message from discord) the temporarily broken one Testing failed:
The test runner encountered an error (Failed to prepare device 'Clone 1 of iPhone 15' for impending launch. (Underlying Error: Timed out trying to boot simulator after waiting 60.00s.))
Test session results, code coverage, and logs: |
Deploying mopro with
|
| Latest commit: |
dbf52f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://25c97380.mopro.pages.dev |
| Branch Preview URL: | https://250-customize-udl-file.mopro.pages.dev |
|
@sifnoc I think if we want to introduce |
Agree. There are at least three places where |
mopro-ffi/src/app_config/ios.rs
Outdated
| if let Ok(info) = fs::metadata(out_dir) { | ||
| if !info.is_dir() { | ||
| panic!("out_dir exists and is not a directory"); | ||
| fn rename_module_map_recursively(bindings_out: &PathBuf) { |
There was a problem hiding this comment.
Could you write a comment explaining what this function does? From context it moves a file named moproFFI.modulemap to module.modulemap in the same directory. It does this exactly once, so it doesn't fully traverse the directory structure if not needed.
| let path = entry.path(); | ||
| if path.is_file() && path.file_name().unwrap() == "moproFFI.modulemap" { | ||
| let dest_path = path.with_file_name("module.modulemap"); | ||
| fs::rename(&path, &dest_path).expect("Failed to rename module map"); |
There was a problem hiding this comment.
If we're only looking for 1 file an explicit break or return might be good here.
There was a problem hiding this comment.
We're looking for multiple as we're normally building for both physical device and the simulator (each corresponding folder has a modulemap). I'll add the comment.
| None, | ||
| false, | ||
| ) | ||
| .expect("Failed to generate bindings"); |
There was a problem hiding this comment.
This is not something we should do in this PR, but in the future we should probably refactor to use anyhow::Result as the return type in all these functions (including main). In all the rust codebases i've worked in (which to be fair is only like 4) I've had to refactor to add anyhow::Result everywhere.
If there's an error case we want to handle explicitly we can always do something like
if let Err(error) = fallible_operation() {
println!("Operation did not succeed because of reason: {}", error);
std::process::exit(1);
}
|
Looks good to me, good PR description too. I left comments about code style only. |
Co-authored-by: Chance <[email protected]>
srcdirectory.