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

[Godot] Don't ignore export presets #2827

Closed
wants to merge 1 commit into from

Conversation

Calinou
Copy link
Contributor

@Calinou Calinou commented Oct 3, 2018

Reasons for making this change:

Export presets are useful to keep around, especially for use in CI/CD.

Export presets are useful to keep around, especially for use in CI/CD.
@KellyThomas
Copy link

KellyThomas commented Oct 4, 2018

@akien-mga You were part of the conversation at #2613

Are there any benefits to keeping these files out of the repo?

@akien-mga
Copy link

It's kind of project specific whether you want to commit the presets to your repo, but yeah, I guess it makes sense to default with them not being ignored.

They can contain things like keystore/release_password and apk_expansion/public_key, so users should just be aware of that before pushing their game source on a public repo.

@shiftkey
Copy link
Member

They can contain things like keystore/release_password and apk_expansion/public_key, so users
should just be aware of that before pushing their game source on a public repo.

In this case I'd like to keep the rules around (whether commented out or not, I'll leave up to y'all to agree on) and properly documented (so people are aware of what this rule represents).

@shiftkey shiftkey changed the title Don't ignore export presets in Godot [Godot] Don't ignore export presets Nov 23, 2018
@shiftkey
Copy link
Member

I'm not sure where we ended up with this - can we close this out unless we've come to a consensus about what changes we should make?

@akien-mga
Copy link

I think it can be merged as is, having the export presets is necessary to keep game-specific config. For open source games, it also makes it easier to package them for Linux distros by generating the data.pck file from the command line based on the "Linux/X11" preset (or any other name they'd set for it).

@Calinou
Copy link
Contributor Author

Calinou commented May 12, 2019

Maybe we could leave the rule commented out, while adding a comment to note that it might contain secrets depending on the configured export presets.

As far as I know, there's no way to revoke Android keystore credentials at all, so people need to be extra careful 🙂

@shiftkey
Copy link
Member

shiftkey commented May 12, 2019

As far as I know, there's no way to revoke Android keystore credentials at all, so people need to be extra careful 🙂

If this is the case I'd prefer the rules remain enabled, but with a comment about sharing if the files have been vetted for sensitive details.

Something like this:

# These files are excluded by default because they can contain credentials and other sensitive
# information, but otherwise may be useful in *[scenarios/reasons here]*
# You can remove these rules and commit these files if you wish to share them, but be careful!
export.cfg
export_presets.cfg

@andyfreer
Copy link

I think from a security perspective anything containing sensitive credentials must be in .gitignore and users warned of the dangers of committing e.g. keystore credentials.

The main issue here is there are useful settings bundled along with those settings that are useful stored in git

Therefore I think the best solution would be to split the credentials parts out into a separate cfg that is ignored and users warned not to commit, then keep this config and remove the .gitignore

@Calinou
Copy link
Contributor Author

Calinou commented Oct 12, 2019

@andyfreer We'll do this in a future Godot version, but since this isn't implemented yet, I'll close this PR for now.

@geekley
Copy link

geekley commented Dec 28, 2023

We'll do this in a future Godot version, but since this isn't implemented yet, I'll close this PR for now.

@Calinou since godotengine/godot#76165 was merged and is out (since 4.1.0 I think?), shouldn't this be reopened? Or would it cause issues with people on earlier versions? Still, if so, it should probably be like:
# export_presets.cfg # Uncomment if not using Godot <version> or later

@Calinou
Copy link
Contributor Author

Calinou commented Jan 1, 2024

shouldn't this be reopened?

Indeed, this can be considered now but I can't reopen this pull request as I previously removed the branch.

That said, remember that Godot 3.x still has this issue and it's still actively supported. As a result, we may want to start shipping separate .gitignore files for Godot 3 and Godot 4.

@micycle8778
Copy link

can that change be backported to godot 3? seems pretty important

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants