Bug 223988 - WebGL conformance tests are missing files due to too widely matching .gitignore
Summary: WebGL conformance tests are missing files due to too widely matching .gitignore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: 222964 223921 30951
  Show dependency treegraph
 
Reported: 2021-03-31 00:47 PDT by Kimmo Kinnunen
Modified: 2021-04-06 04:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (346.99 KB, patch)
2021-03-31 00:52 PDT, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (345.88 KB, patch)
2021-04-05 23:43 PDT, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2021-03-31 00:47:51 PDT
WebGL conformance tests are missing files due to too widely matching .gitignore

.gitignore contains:
build/

The test suite contains:
LayoutTests/webgl/1.0.x/conformance/ogles/GL/build/build_001_to_008.html


The .gitignore probably is not what was intended. Other projects have had to work around the issue, too.
Comment 1 Kimmo Kinnunen 2021-03-31 00:52:58 PDT
Created attachment 424742 [details]
Patch
Comment 2 Jeff Johnson 2021-03-31 04:54:31 PDT
I believe that https://bugs.webkit.org/show_bug.cgi?id=30951 which has been closed for over 11 years was erroneously added to the "Blocks" list.
Comment 3 Kimmo Kinnunen 2021-03-31 04:55:36 PDT
It's the source of this bug.
Comment 4 Alexey Proskuryakov 2021-03-31 09:20:51 PDT
Comment on attachment 424742 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424742&action=review

> .gitignore:7
>  .directory
> -build/
> -!/PerformanceTests/Speedometer/**/build/
> +/build/

This also makes `build` directories under Websites/browserbench.org/Speedometer2.0 ignored. I'm not 100% certain, but in the case of Websites folder, that may be wrong. Can you confirm with Ryosuke before landing?

In fact, it's not even clear to me why `build` directories under PerformanceTests are ignored, as there is checked in content there.
Comment 5 Kenneth Russell 2021-03-31 09:28:55 PDT
Comment on attachment 424742 [details]
Patch

Great find Kimmo!
Comment 6 Kimmo Kinnunen 2021-04-01 02:23:44 PDT
> This also makes `build` directories under Websites/browserbench.org/Speedometer2.0 ignored. I'm not 100% certain, but in the case of Websites folder, that may be wrong. Can you confirm with Ryosuke before landing?


I think the intention of my change is to ensure that the build/ directories of Websites folders *not* ignored. This is opposite of what you wrote. Can you double-check if you wrote what you thought?

https://git-scm.com/docs/gitignore

> If there is a separator at the beginning or middle (or both) of the pattern, then the pattern is relative to the directory level of the particular .gitignore file itself. Otherwise the pattern may also match at any level below the .gitignore level.




I think all these are intended to be here, if some of them are intended:

find . -iname 'build' 

./LayoutTests/webgl/1.0.3/resources/webgl_test_files/conformance/ogles/GL/build
./PerformanceTests/Speedometer/resources/todomvc/labs/architecture-examples/react/bower_components/director/build
./PerformanceTests/Speedometer/resources/todomvc/architecture-examples/vuejs-cli/build
./PerformanceTests/Speedometer/resources/todomvc/architecture-examples/react/node_modules/director/build
./Websites/browserbench.org/Speedometer2.0/resources/todomvc/labs/architecture-examples/react/bower_components/director/build
./Websites/browserbench.org/Speedometer2.0/resources/todomvc/architecture-examples/vuejs-cli/build
./Websites/browserbench.org/Speedometer2.0/resources/todomvc/architecture-examples/react/node_modules/director/build

Since there was an explicit non-ignore pattern for one of these directories, I assume this intention is just missing for the other one by mistake.
Comment 7 Alexey Proskuryakov 2021-04-01 19:04:26 PDT
> I think the intention of my change is to ensure that the build/ directories
> of Websites folders *not* ignored. This is opposite of what you wrote. Can
> you double-check if you wrote what you thought?

You are of course right. It's still a change for Speedometer that's not directly related to OpenGL and possibly not intended - yet likely desirable anyway. I still think that a quick confirmation from Ryosuke would be good.
Comment 8 Kimmo Kinnunen 2021-04-05 23:43:55 PDT
Created attachment 425246 [details]
Patch
Comment 9 Kimmo Kinnunen 2021-04-05 23:56:39 PDT
Moved the .gitignore change to a separate bug 224227, as reviewing it seems to take more time.
If bots turn green, taking the liberty to land the tests as reviewed by Alexey, since the patch was already r+'ed.
Comment 10 EWS 2021-04-06 04:46:33 PDT
Committed r275505: <https://commits.webkit.org/r275505>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425246 [details].
Comment 11 Radar WebKit Bug Importer 2021-04-06 04:47:20 PDT
<rdar://problem/76262297>