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

fix: don't cancel build when default romfs directory not found #51

Closed
wants to merge 1 commit into from

Conversation

fekie
Copy link

@fekie fekie commented Nov 27, 2023

When creating a project with $ cargo 3ds new foo, an empty romfs directory is created. Because this folder is empty, it will not be added to git. Because of this, when cloning from a repo of a project created with cargo-3ds, the romfs directory will not download and $ cargo 3ds build will fail because it is missing.

The change in this pull request changes it to where if it doesn't find a romfs folder at the default directory, it will create it at the default directory and proceed.

It may be better to just skip creating it and proceeding, but I don't yet know the internals of the program that well.

If you want me to change it a bit or do it another way I'd be happy to contribute.

@fekie fekie requested a review from a team as a code owner November 27, 2023 22:20
@Meziu
Copy link
Member

Meziu commented Nov 28, 2023

Oops, I totally get the issue. I'm quite unsure whether re-creating the folder is the right call though, since that warning is specifically made to inform the user that the folder is missing (and that could be problematic if there was supposed to be a romfs folder there...). In short, this type of solution could affect users that aren't connected to the original issue, which is git ignoring the folder when committing.

Could we instead find a solution for that? An easy way to solve the problem would be to add an empty (or "dummy") file within the romfs folder when running $ cargo 3ds new, but there are also other possibilities.

@ian-h-chamberlain any thoughts about this?

@fekie
Copy link
Author

fekie commented Nov 28, 2023

Oops, I totally get the issue. I'm quite unsure whether re-creating the folder is the right call though, since that warning is specifically made to inform the user that the folder is missing (and that could be problematic if there was supposed to be a romfs folder there...). In short, this type of solution could affect users that aren't connected to the original issue, which is git ignoring the folder when committing.

Could we instead find a solution for that? An easy way to solve the problem would be to add an empty (or "dummy") file within the romfs folder when running $ cargo 3ds new, but there are also other possibilities.

@ian-h-chamberlain any thoughts about this?

A readme in it might be nice. Like just something that describes what it's used for (and maybe while to file exists). Or something like a file named "put x files here" (I think this is the more common route).

@ian-h-chamberlain
Copy link
Member

Oops, I totally get the issue. I'm quite unsure whether re-creating the folder is the right call though, since that warning is specifically made to inform the user that the folder is missing (and that could be problematic if there was supposed to be a romfs folder there...). In short, this type of solution could affect users that aren't connected to the original issue, which is git ignoring the folder when committing.

What if we allowed the user to decide the behavior? We could have something like this:

[package.metadata.cargo-3ds]
romfs_dir = { path = "romfs", error-if-missing = true }

Perhaps the default could be to just print a warning, but opting in for a hard error could address the concern you have about if a folder is missing but supposed to be there?

A readme in it might be nice. Like just something that describes what it's used for (and maybe while to file exists). Or something like a file named "put x files here" (I think this is the more common route).

This isn't a bad idea, it would ensure the directory exists for git tracking purposes while also explaining how it works a little bit. We could also add a flag to cargo 3ds new to skip generating it if we wanted, I suppose.

@Meziu
Copy link
Member

Meziu commented Dec 2, 2023

A readme in it might be nice. Like just something that describes what it's used for (and maybe while to file exists). Or something like a file named "put x files here" (I think this is the more common route).

This isn't a bad idea, it would ensure the directory exists for git tracking purposes while also explaining how it works a little bit. We could also add a flag to cargo 3ds new to skip generating it if we wanted, I suppose.

I believe this is the simplest and cleanest solution for the problem. The user could always delete it if they don't want it.

@Meziu Meziu mentioned this pull request Apr 26, 2024
@Meziu Meziu closed this May 3, 2024
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.

3 participants