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

Allow creating .gitignore and .gitattributes when creating a new project #42447

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Oct 1, 2020

Implements and closes godotengine/godot-proposals#1813.

Many people either forget to make .gitignore files, or make them incorrectly, or make them correctly but not until after they committed files that shouldn't be committed. There was also some concern about possible confusion with the .godot/ folder. We can solve all of these problems by having Godot automatically create a .gitignore file.

This PR also has Godot automatically create a .gitattributes file. It's a subset of the one in the main repo. This doesn't include any manual definitions for binary files, but I think it's safe to assume Git >2.10 now (2.11 came out in 2016).

For people who aren't using Git, these files are useless, but also they are harmless and extremely small (total 116 bytes), and there is a dropdown menu where you can disable their creation. In the future, this dropdown menu could be expanded to have more types of metadata for different version control systems.

Note: This gitignore doesn't include .translation, so this PR expects that #42392 will be merged (but the PRs shouldn't conflict).

This can be cherry-picked to 3.x if desired, but we will have to include the rest of the 3.x ignores too.

$ cat .gitignore
# Godot 4+ specific ignores
.godot/
$ cat .gitattributes
# Normalize EOL for all files that Git considers text files.
* text=auto eol=lf

EDIT: The files are now optional, they can be disabled in the project creation dialog:

Screenshot from 2020-11-13 03-58-54

@timothyqiu
Copy link
Member

AFAIK, it's a bit strange to require eol=lf on Windows.

text=auto alone can make sure that line endings are normalized to LF in the repository. eol=lf makes all text files in the working directory to use LF line endings, which is unnatural on Windows.

@pingwindev
Copy link

I'm opposed to having possibly useless files as a default in each single project directory, no matter the size. On the other hand, the majority probably uses git anyway or doesn't care that much about some clutter. But still, Godot is open to be used with different VCS and I find it bothersome of an application to assume for me, what I might need.

I'd rather like to see this as a part of an optional "project template" setup to pre-create most common project structures with VCS integration or not.

@Calinou
Copy link
Member

Calinou commented Oct 1, 2020

I'm in favor of this, since these files will be hidden by default on macOS and Linux. On Windows, we might be able to set the "hidden" attribute ourselves in the future.

While it is possible to use other version control systems with Godot, the overwhelming majority of users I know of use Git. This goes from gamejam developers to medium-sized indie studios 🙂

@Xrayez
Copy link
Contributor

Xrayez commented Oct 1, 2020

I have a Python package gdgen which allows you to generate C++ modules with git initialized, but that's only because it's something which is not provided by Godot officially (I cannot recall whether it's possible to override this in gdgen currently, if not, then that should be a bug).

On the other hand, we have plugins like https://github.com/godotengine/godot-git-plugin semi-officially maintained:

Git implementation of the Godot Engine VCS interface in Godot.

Godot seems to have the API for implementing various VCS providers. So Godot can decide to officially support git on that level.

I'd rather like to see this as a part of an optional "project template" setup to pre-create most common project structures with VCS integration or not.

Semi-related proposal: godotengine/godot-proposals#1481. I mean, this could be potentially made configurable via the project manager checkboxes:

  • Project name: Godot
  • Project path: /home/godot/projects/new_project
  • Initialize with git (potentially only add this option when git is detected in the OS).

@aaronfranke
Copy link
Member Author

@timothyqiu As discussed here, it's best to use LF everywhere with Godot.

@artism90 I forgot to mention that it is extremely trivial to delete these files if you don't want them.

@akien-mga
Copy link
Member

I'd rather like to see this as a part of an optional "project template" setup to pre-create most common project structures with VCS integration or not.

I agree. There should be a possibility to initiate an actual Git repo with those files (like e.g. cargo new would do for a Rust project), and possibly adding support for other popular VCS systems with their own relevant configuration files.

@aaronfranke
Copy link
Member Author

Maybe there should be a checkbox in editor settings for this?

Also, this reminds me, it would be nice if there was a way to access editor settings in the project manager.

@timothyqiu
Copy link
Member

timothyqiu commented Oct 2, 2020

As discussed here, it's best to use LF everywhere with Godot.

I think what config Godot uses and what config a godot project uses are two different things.

Unlike line endings in the repository, using CRLF line ending in the working directory won't cause any trouble. It's just a matter of the repository owner's taste.

The Godot team is Linux native. It's reasonable for the team to choose forcing LF everywhere to minimize the trouble in its own repository, even though the choice is kind of an anti-pattern, allowing to copy the directory directly between Linux and Windows instead of using Git to fetch the source code.

Rust also uses * text=auto eol=lf in the Rust repository itself, and cargo new generates .gitignore but not .gitattributes, leaving the option to the user.

@aaronfranke
Copy link
Member Author

aaronfranke commented Oct 2, 2020

I updated this PR so that these files are now optional, and it's easy to disable their creation in the editor settings. I also took the opportunity to allow disabling the creation of default_env.tres, since developers who work primarily in 2D usually don't need an environment in their projects (see this PR).

With everything disabled, an empty project only has project.godot (required), icon.png, icon.png.import, and the .godot/ folder.

@aaronfranke
Copy link
Member Author

I discussed this with some people on Discord and it seems that there are indeed some people who would find this annoying, so I changed this PR to have these new settings disabled by default.

In the future if it's decided that we should have this setting enabled by default, it would be easy to make a follow-up PR for that, but in the meantime this PR in its current state can only benefit people since the default behavior is the same.

@realkotob
Copy link
Contributor

realkotob commented Oct 3, 2020

I am really happy about this PR, but I would like to argue against having it Off by default.

It doesn't matter if there are some people who find it annoying, the majority of people use git for version control, and it makes more sense to provide sensible defaults that cater to most users, not the outliers.
It sucks for those "some people" but that's what sensible defaults mean.

Are there other scenarios reasons against a .gitignore file? (Other than "we don't use git for version control")

Also note that the Cocos Creator engine (widely popular) also provides a .gitignore by default on project creation, so there is precedent.

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 8, 2020

It sucks for those "some people" but that's what sensible defaults mean.

It is not sensible, though, that this option is hidden in the editor settings, in an unrelated to VCS section. This should be a project creation step with an optional editor top menu item to recreate those files after the initial project creation if they have been omitted (or if it is an existing project).

Which VCS somebody uses is very situational and personal thing, and just because there is a trend in OSS community doesn’t mean we should make this harder to configure. More than that, this setting may vary per project inside of the same team.

So unless this is an option like “Create git repository” available at project creation, then this is an unwelcome change. With a room to introduce other popular VCSs later, of course. The default state can be made configurable in the editor settings, if needs be.

@realkotob
Copy link
Contributor

@pycbouh I'm fine with making it an option during project creation (and remembering the choice per-user locally for future projects). As long as I can make it init git by default I don't really care how visible the option is.

@aaronfranke
Copy link
Member Author

@pycbouh Adding these options absolutely does not make any VCS system harder to configure. With non-Git VCS systems, even if these checkboxes were enabled by default, the files are harmless and super easy to delete.

I disagree that this change is unwelcome if there isn't also a way to initialize a Git repository, I think these are separate issues. I made this PR to try and solve real world problems people have, where people put a Godot project on GitHub and they are missing these important files. On the other hand, it is impossible for people to put a project in a GitHub repository without a Git repository, so people needing Godot to create a Git repository is not the real world problem I'm trying to solve with this PR, and is a separate issue.

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 8, 2020

@aaronfranke You've misunderstood me completely...

My point was about making this new addition configurable at the project creation screen instead of hiding it somewhere in the editor settings, and in a section that has nothing to do with VCS at that. It was suggested by 3 other people before me that we should be able to tick this as we are making a new project, and putting this setting into a menu inaccessible unless you open a project is making it harder to configure than it needs to be. I am not against adding these options, if you make them easy to disable for a particular project.

Which is why I suggest adding a toggle saying something like "Create/Initialize git repository" to the project creation window, as well as adding this as an action to the top editor menu (say, below the "Project" item) so people can initialize the repository for existing projects as well. And of course this should be implemented in a way that would allow adding other VCSs in the future. So it's not so much "Init git" but "Init a VCS" with git as the only currently available option. Also that option as well as whether or not do it by default at all should be remembered and configured for future projects as asheraryam says.

And I wasn't saying that we must initialize the repo for this PR to be welcome. I wasn't making any distinctions between simply creating those two files and fully initializing the repo. If you'd like to just create two files in the first iteration of this, do that. I was saying that this PR is in my opinion unwelcome if you don't provide an option to prevent this action from happening from the project creation screen, that's it.

@aaronfranke
Copy link
Member Author

aaronfranke commented Nov 9, 2020

@pycbouh I'm concerned that this is a slippery slope that could lead to the project creation popup having tons of checkboxes, as it is now your idea would add four of them (init git repo, create gitignore, create gitattributes, create default environment). I think a better idea would be to allow editing the editor settings from inside the project manager, so that you can enable/disable the creation of these files without first opening a project (and maybe later also allow creating a Git repo).

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 9, 2020

First of all, everything Git related should be under one checkbox here. There is almost no need for such granularity as you describe. In fact it's that granularity that can be put into the editor settings, but the action should be toggled from the creation screen with just one checkbox. And when we add more VCSs, it will be just a drop list alongside that checkbox.

Secondly, no need fear-mongering with the slippery slope, we are fully capable of making a judgement on what settings should or shouldn't go there. They can even exist in a collapsible section, so that they don't take space unless you know you want to configure them.

Ultimately, this is a setting that differs for each individual project, it makes sense to decide on it the moment you create them, not fiddle with the whole set of editor settings just to create a single project. That's an unnecessarily complicated way to handle a forced feature that has nothing to do with Godot. It's very presumptuous to assume that because a lot of people use Git, others must be inconvenienced and required to seek out a hidden setting or to remove files afterwards. I don't want to say that there are bad intentions, but many comments here assume that it's an okay approach because it aligns with their behavior when using this software. I don't think that ignoring the needs of so called "some people" is a correct way to handle this feature.

@twaritwaikar
Copy link
Contributor

twaritwaikar commented Jul 6, 2021

One of the reasons why https://github.com/godotengine/godot-git-plugin was made into a different plugin was that the editor was supposed to be kept as VCS agnostic as possible.

However, it is true that most users use Git, so the initial implementation of the VCS interface was done for Git. The Git plugin actually takes care of creating a .gitignore and a .gitattributes file with contents as mentioned here if those are not already present, as well as takes care of initialising the file system with a .git folder (akin to running git init), which is kind of an obvious thing to do if the user is wanting to create Git metadata files.

So should this functionality just be implemented by the VCS interface implementations (plugins) or is it fine if the editor implements the same thing again? Because using Git specific terms in the plugin implementation is expected but using them in the editor break that kind of VCS agnosticism built in it.

@aaronfranke
Copy link
Member Author

@ChronicallySerious If you take a closer look, there is a dropdown menu that allows selecting either Git or None. Any other metadata files for other version control systems can easily be added. However, I haven't used anything other than Git.

Someone who uses Git with Godot may use a separate client and therefore not want full blown support in the editor, but they still need these metadata files, and it can't be expected that third-party clients create them. This means that either Godot does it, or users have to hunt them down manually, or they forget.

Another possibility is that users start with command-line Git and only install the Godot Git plugin later. In which case, it's too late for the plugin to create these files - some gitignored files may have already been committed and have to be deleted manually.

@aaronfranke aaronfranke force-pushed the gitignore-create branch 3 times, most recently from 28d70a7 to 12616fc Compare July 26, 2021 05:41
@aaronfranke aaronfranke force-pushed the gitignore-create branch 2 times, most recently from 8803cae to d48bfa9 Compare August 1, 2021 16:48
@reduz
Copy link
Member

reduz commented Aug 30, 2021

What is the resolution on this one? do we have full consensus to merge? Also devault_env.tres and related should be removed, it's no longer needed in Godot 4.

@aaronfranke
Copy link
Member Author

aaronfranke commented Aug 30, 2021

@reduz Some people are against it but most are in favor.

As for removing default_env.tres, if we are doing that, then we should do that first. We can keep this PR on hold for now until that's done and then I can rebase this PR later.

EDIT: No, default_env.tres is still required. It's not necessary in the editor due to the preview environment, but for running a project without a default environment with only a Camera3D in it just displays a gray sky. So we still need this. I think having a checkbox for it in the project creation popup (as in this PR) is better than leaving that space empty in the project creation popup.

@aaronfranke aaronfranke force-pushed the gitignore-create branch 4 times, most recently from e6b73db to d33044e Compare September 20, 2021 06:19
@Calinou
Copy link
Member

Calinou commented Sep 20, 2021

EDIT: No, default_env.tres is still required. It's not necessary in the editor due to the preview environment, but for running a project without a default environment with only a Camera3D in it just displays a gray sky. So we still need this. I think having a checkbox for it in the project creation popup (as in this PR) is better than leaving that space empty in the project creation popup.

With the preview sun and sky system, this is meant to be resolved differently. For example, we could print a warning message by scanning the scene tree for the lack of WorldEnvironment node in debug builds (only if there's at least 1 Node3D in the scene). The same can be done for the lack of a Camera3D node.

Also allow creating these files later, and also allow disabling creating the default environment in editor settings.
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Approved in PR review meeting.

@akien-mga akien-mga merged commit c5ab537 into godotengine:master Nov 23, 2021
@akien-mga
Copy link
Member

Thanks!

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.

Allow creating .gitignore and .gitattributes files when creating a new project