Fix .gitignore files ignore part of the repo. #47212

zorbathut · 2021-03-21T06:43:15Z

At the moment, if you download the Godot release .zip, decompress it into a new git repo, add the entire contents, clean the git repo, and try to build Godot, the build is broken. Turns out Godot has several critical files (and a few less-critical files) included in its .gitignore.

From my perspective, the most important ones are the entire modules/mono/editor/GodotTools/GodotTools/Build/ directory, which breaks the Mono build if they don't exist. These are filtered out due to the build/ entry. As near as I can tell, that entry is part of the .gitignore template, and Godot does not actually use any directory named build/. So I just took that line out. This may have been the wrong decision if Godot's build process is markedly different on other operating systems.

The second most important ones are a bunch of Vulkan files for various Apple operating systems:

misc/dist/ios_xcode/godot_ios/vulkan/icd.d
misc/dist/osx_template.app/Contents/Resources/vulkan/icd.d
misc/dist/osx_tools.app/Contents/Resources/vulkan/icd.d

In all of these cases it's the .d suffix causing problems. I added a .gitignore in misc/dist to cover icd.d, mostly because it was easy, but it's possible this should be three separate .gitignore's in the exact relevant directory. Alternatively: does Godot even use the .d extension? If it does, it's probably on Linux, and I don't have a good way to do a Linux build right now. But if it doesn't, that could probably just be removed from the .gitignore.

The last was the updown.png file in the default theme, removed because it happens to match gcov/lcov code coverage files, change added in 5b4d74e. I just unfiltered all the .png files in that directory. This is the fix I like the least - this really should be filtering out gcov/lcov in some other way, not ignoring a pile of generically-named files throughout the entire repository - but I have no idea what directory names gcov/lcov uses.

Obviously I am dissatisfied with these fixes.

On the other hand, now I can build Godot in a fresh Git repo. So . . . it's better, at least.

zorbathut · 2021-03-21T06:51:51Z

It also occurs to me that (given that this has happened at least three times) this should probably be a build test. Maybe under Static Checks? Let me know if that's desirable and I'll get it in.

aaronfranke · 2021-03-21T08:51:56Z

From my perspective, the most important ones are the entire modules/mono/editor/GodotTools/GodotTools/Build/ directory, which breaks the Mono build if they don't exist. These are filtered out due to the build/ entry.

Can't confirm, these files aren't ignored and are committed. I also tried deleting them, committing, and restoring them, and they show up as additions. Can you give us details of your exact environment? Maybe it's broken on Windows or Mac due to case insensitivity? I only tested on Linux as of writing this comment.

zorbathut · 2021-03-21T18:11:03Z

Yeah I'd bet actual money this is a case-insensitive-filesystem deal (I actually meant to write that in and forgot, mea culpa.) The .gitignore lists build, the actual directory is Build. I'm on Windows 10, NTFS partition, I assume nothing else is relevant here :)

(thinking this over, this would also make it a lot harder to test in the Static Checks build phase)

akien-mga · 2021-03-23T11:49:32Z

scene/resources/default_theme/.gitignore

+# Don't ignore some graphics. Overrides Godot's global gitignore (specifically the part that filters out code coverage.)
+!*.png


Could be solved in the main .gitignore by making the code coverage ignores absolute.

CC @qarmin, are those always generated in the root folder?

Not always, but depends where user execute specific commands.
I think that only sensible directories in which user may execute generation of html files are root(/) or bin folder(which is hidden by default in git.) - to generate coverage report, Godot binary must be executed.

So changing this records to point to absolute path are fine to me

MachineIndependent/ godot.info amber.png emerald.png glass.png ruby.png snow.png updown.png gcov.css

Would those files show up in the actual root directory, or in a subdirectory? Alternatively, can you give me the magic command needed to generate that coverage report so I can test it? :)

For now I cannot test how exactly it works but this are steps how to get coverage report(works only on Linux and 4.0 branch):

sudo apt install gcc gcov lcov clang # Some may be not available scons p=linuxbsd use_coverage=yes bin/godot.linuxbsd.tools.64s lcov --directory ./ --capture --output-file godot.info genhtml godot.info --ignore-errors source lcov --zerocounters # To clear results xdg-open index.html # opens results

Unfortunately it looks like this doesn't function under VirtualBox, which means I can't test it either.

However, looking at the commandline, it looks like you can specify a directory for this stuff to end up in. Maybe we should just ignore an lcov or coverage directory, then tell people to put their coverage data in there instead of in the root directory?

akien-mga · 2021-03-23T11:51:56Z

It also occurs to me that (given that this has happened at least three times) this should probably be a build test. Maybe under Static Checks? Let me know if that's desirable and I'll get it in.

That would be overkill IMO, we shouldn't have to test this on every single PR for a somewhat niche use case which wouldn't be broken often.

Overreaching .gitignore patterns should definitely be fixed, but it's sufficient to fix them when we find them IMO.

zorbathut · 2021-04-02T03:41:08Z

Ping. Advice requested for what to do with the coverage segment?

I'll just make the coverage directory change mentioned in #47212 (comment) if there's no further commentary, I suppose.

zorbathut · 2021-04-18T03:49:45Z

Change made, let me know if you want it changed back or with a different solution.

zorbathut · 2022-10-21T19:10:14Z

Alright, back to this!

In the intervening time, most of the existing issues were fixed at some point . . . replaced with a new set of issues. I've added fixes for those, but I'd like to reiterate that I think this should be part of the precommit tests.

For what it's worth, this is important to me because I would like the Godot sourcecode included per-project since I know I'll be making changes to it. I'd like to be able to just copy the latest Godot into a directory and have it work; this bit me for a while when I screwed it up the first time, now it's just a pain point whenever I update.

Please let me know if there's a better solution to this.

akien-mga · 2023-02-02T08:27:24Z

thirdparty/.gitignore

+# Directories caught by the root .gitignore's buildfile list
+!embree/common/simd/arm/
+!libpng/arm/
+!libtheora/x64/


These don't seem needed anymore.

The x64 one is no longer needed. The arm exclusions are still needed in some form, they're matching .gitignore:250:

[Aa][Rr][Mm]/

I'll move them into the main file and follow the x64 pattern.

akien-mga · 2023-02-02T08:28:50Z

misc/dist/.gitignore

@@ -0,0 +1,2 @@
+# Directory caught by the dependency .gitignore
+!ios_xcode/godot_ios/vulkan/icd.d


I would just put this in the main .gitignore with an absolute path.

vulkan/icd.d files can be removed, we use static libs for MoltenVK, so these are useless.

I'm happy to delete 'em if you want; this feels out of my paygrade, if that makes sense, but if it's the right choice I can just do it.

zorbathut · 2023-02-03T14:08:22Z

Alright, moved them into the root .gitignore file.

For reference, the Linux incantation to find issues like this is:

git -c core.ignorecase=true check-ignore --no-index **/*

although for obvious reasons you need to run this before doing a build or you're going to find a lot of ignored files.

akien-mga · 2023-02-17T12:49:11Z

I squashed the commits, moved the ignores to the main .gitignore as requested, and removed the unneeded icd.d in the iOS template.

akien-mga · 2023-02-17T13:08:44Z

Thanks!

akien-mga · 2023-08-29T09:57:44Z

Cherry-picked for 3.6.

zorbathut · 2023-08-29T10:09:06Z

Gentle ping to point someone at #78908 to fix these issues once and for all :V

akien-mga · 2023-09-06T11:16:19Z

Cherry-picked for 3.5.3.

zorbathut requested review from a team as code owners March 21, 2021 06:43

Calinou added the bug label Mar 21, 2021

akien-mga reviewed Mar 23, 2021

View reviewed changes

aaronfranke requested a review from akien-mga April 18, 2021 05:45

pouleyKetchoupp added this to the 4.0 milestone May 8, 2021

zorbathut force-pushed the pr_gitignore branch from 7707077 to 296bbd3 Compare October 21, 2022 19:06

akien-mga reviewed Feb 2, 2023

View reviewed changes

Fix .gitignore ignores part of the committed repo.

37d6892

akien-mga force-pushed the pr_gitignore branch from aad1d2d to 37d6892 Compare February 17, 2023 12:48

akien-mga approved these changes Feb 17, 2023

View reviewed changes

akien-mga added topic:buildsystem cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Feb 17, 2023

akien-mga merged commit 1e3f42f into godotengine:master Feb 17, 2023

zorbathut deleted the pr_gitignore branch February 17, 2023 21:16

madmiraal mentioned this pull request May 5, 2023

Import Godot 3.4.5 RebelToolbox/RebelEngine#5

Merged

zorbathut mentioned this pull request Jul 1, 2023

Add static check for overzealous .gitignores and fix an example of such. #78908

Merged

akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 29, 2023

akien-mga added the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Aug 29, 2023

akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Sep 6, 2023

akien-mga changed the title ~~Fix: .gitignore files ignore part of the repo.~~ Fix .gitignore files ignore part of the repo. Sep 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix .gitignore files ignore part of the repo. #47212

Fix .gitignore files ignore part of the repo. #47212

zorbathut commented Mar 21, 2021

zorbathut commented Mar 21, 2021

aaronfranke commented Mar 21, 2021 •

edited

Loading

zorbathut commented Mar 21, 2021

akien-mga Mar 23, 2021

qarmin Mar 23, 2021 •

edited

Loading

zorbathut Mar 23, 2021

qarmin Mar 23, 2021

zorbathut Mar 24, 2021

akien-mga commented Mar 23, 2021

zorbathut commented Apr 2, 2021

zorbathut commented Apr 18, 2021

zorbathut commented Oct 21, 2022

akien-mga Feb 2, 2023

zorbathut Feb 3, 2023

akien-mga Feb 2, 2023

bruvzg Feb 3, 2023

zorbathut Feb 3, 2023

zorbathut commented Feb 3, 2023

akien-mga commented Feb 17, 2023

akien-mga commented Feb 17, 2023

akien-mga commented Aug 29, 2023

zorbathut commented Aug 29, 2023

akien-mga commented Sep 6, 2023

		# Don't ignore some graphics. Overrides Godot's global gitignore (specifically the part that filters out code coverage.)
		!*.png

		@@ -0,0 +1,2 @@
		# Directory caught by the dependency .gitignore
		!ios_xcode/godot_ios/vulkan/icd.d

Fix .gitignore files ignore part of the repo. #47212

Fix .gitignore files ignore part of the repo. #47212

Conversation

zorbathut commented Mar 21, 2021

zorbathut commented Mar 21, 2021

aaronfranke commented Mar 21, 2021 • edited Loading

zorbathut commented Mar 21, 2021

Choose a reason for hiding this comment

qarmin Mar 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Mar 23, 2021

zorbathut commented Apr 2, 2021

zorbathut commented Apr 18, 2021

zorbathut commented Oct 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zorbathut commented Feb 3, 2023

akien-mga commented Feb 17, 2023

akien-mga commented Feb 17, 2023

akien-mga commented Aug 29, 2023

zorbathut commented Aug 29, 2023

akien-mga commented Sep 6, 2023

aaronfranke commented Mar 21, 2021 •

edited

Loading

qarmin Mar 23, 2021 •

edited

Loading