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.

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

General refactoring and cargo-new #36

Merged
merged 7 commits into from
Jul 12, 2023
Merged

General refactoring and cargo-new #36

merged 7 commits into from
Jul 12, 2023

Conversation

Meziu
Copy link
Member

@Meziu Meziu commented Jun 30, 2023

Closes #29
Closes #14
Would also resolve issues such as: discussions#127

This PR includes support for the cargo 3ds new command, as well as a small refactoring of the project needed to implement the new functionality.

List of changes:

  • cargo 3ds new is now recognized as an official command. Once run for a new directory it will create a new custom project. The differences between cargo 3ds new and a standard cargo new for now are simple: automatically adds ctru-rs (via git repository for now) as a dependency of the project, writes a completely custom main.rs which uses the ctru::prelude to run a simple "Hello, World!" program, and adds an empty romfs folder + Cargo.toml configuration to get RomFS working more easily.
  • As my mention of Is it necessary to modify RUSTFLAGS in cargo-3ds?  #14 can tell, modifying the RUSTFLAGS is now not needed, and that call has been removed. This is thanks to those same changes in the linking mechanism within rustc that let us get away with link ordering problems. As such, libctru linking isn't specified twice anymore, instead it only depends on the calls within the ctru-sys build script.
  • Since cargo-new is the first subcommand we officially support which does not compile any code, many changes have taken place within cargo-3ds to ensure more flexibility. Now the cargo arguments are added based on the command and every subcommand can specify an "after-cargo" callback to work with external functionality (e.g. build creates the .3dsx file in its callback, as well as new does its changes there too).

I will update the ctru-rs wiki shortly after this PR merges to briefly explain the new functionality.

With this PR I intend to officially release cargo-3ds via crates.io. I haven't had any issues with cargo-3ds for months now, and the support has only grown bigger. We can start publishing this tool as Rust3DS' first official crate, since its clearly the most mature.

Edit: I forgot to say why I decided not to implement cargo 3ds init as well. While it wouldn’t have been hard to implement it the same way as new, it posed immense issues in how we would got about actually modifying the context. cargo-init, the official one, takes an existing directory and adds to it all the files and config that would normally appear when running cargo-new. However, it does not modify existing files that correspond to the project files (e.g. if src/main.rs already exists, it skips over its initialisation). This means that by the end of the command, the folder will look more or less exactly as it would after running new, but in this case, we aren’t certain that the files present are brand new, and there’s no way of knowing that. Modifying the Cargo.toml or making a custom main.rs replacing the original one would be disastrous. The only way to solve such an issue would be to completely rewrite cargo-init, but that seems a bit out of scope for this tool.

@Meziu Meziu requested a review from a team as a code owner June 30, 2023 09:43
@Meziu Meziu merged commit 6c22e50 into master Jul 12, 2023
@Meziu Meziu deleted the feature/cargo-new branch July 14, 2023 14:57
Copy link
Member

@ian-h-chamberlain ian-h-chamberlain left a comment

Choose a reason for hiding this comment

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

Just taking a peek at what got merged for this, looks good to me!

}
}
}
";
Copy link
Member

Choose a reason for hiding this comment

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

tiny nit: these could be r#"raw strings"# if you wanted to avoid escaping

fn callback(&self) {
// Commmit changes to the project only if is meant to be a binary
if self.cargo_args.args.contains(&"--lib".to_string()) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

for libs we could probably add the ctru-rs dependency but leave everything else alone. Not really saving as much time vs the rest though

fs::File::open(&toml_path)
.unwrap()
.read_to_string(&mut buf)
.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

could avoid this reading step and extra buffer if you open with File::options().append(true) instead of read, copy + write.

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.

cargo 3ds new and init commands Is it necessary to modify RUSTFLAGS in cargo-3ds?
2 participants