Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

Add dummy files to preserve vendor directories #172

Merged
merged 7 commits into from
Feb 9, 2022

Conversation

0xd61
Copy link
Contributor

@0xd61 0xd61 commented Feb 6, 2022

When using go mod vendor, directories without any go files are automatically deleted. This causes errors because the cgo includes cannot be found anymore. This PR adds dummy files to prevent go vendor from deleting these directories.
For reference go-gl/glfw does the same: go-gl/glfw@0e1e096

golang/go#26366

@dertseha
Copy link
Member

dertseha commented Feb 7, 2022

That's a good extension.
May I ask you to replace the wording of "dummy" with something more fitting, such as "placeholder".
See also reasoning here and here.

@0xd61
Copy link
Contributor Author

0xd61 commented Feb 7, 2022

I renamed the files to govendorkeep (similar to how gitkeep is used in git to commit empty directories). However I don't know what to do about the failed tests. I tested everything locally and it worked fine.

wrapper_cgo_hack.go Outdated Show resolved Hide resolved
Co-authored-by: Christian Haas <christian.haas@sevensuns.at>
@dertseha
Copy link
Member

dertseha commented Feb 7, 2022

I renamed the files to govendorkeep (similar to how gitkeep is used in git to commit empty directories). However I don't know what to do about the failed tests. I tested everything locally and it worked fine.

This probably comes from the now new package to not use cgo in case of test is invoked. You should be able to recreate the issue by entering one of the C++ directories manually and run go test . there.
This may be solvable by putting an additional import "C" into the keep files as well, though I'm not sure if this then annoys the compiler again for not being used... You might have to add some additonal tricks here. And/Or find a way so that go test ignores these directories.

@0xd61
Copy link
Contributor Author

0xd61 commented Feb 8, 2022

Looks like, the whole directories must be excluded on linting. Should be fixed now.

@0xd61
Copy link
Contributor Author

0xd61 commented Feb 8, 2022

Does the linter in github actions use a different config file than .golangci.yml? I've tested it locally with the docker command documented on the golangci-lint website docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:v1.44.0 golangci-lint run -v and it worked fine.

@dertseha
Copy link
Member

dertseha commented Feb 8, 2022

Hm. The action does use an older version of golangci-lint (v.1.28.3) . Perhaps the older version doesn't support excluding directories.

Simply upgrading the action might add way more issues, as the config is still in the "old" format of simply enabling "all" linters. What will be necessary is to instead of enabling all, explicitly enable only those that were in the used version so far, then the version could be upgraded.
However, you wrote that you had no issues at all and perhaps we are lucky. How about trying out upgrading the linter in the action and then run it.

@0xd61
Copy link
Contributor Author

0xd61 commented Feb 8, 2022

I meant skipping the directories went fine. But I will try to update the linter and the config.

@dertseha dertseha merged commit f8ed29c into inkyblackness:main Feb 9, 2022
@dertseha
Copy link
Member

dertseha commented Feb 9, 2022

Great - working now!
Thank you for your contribution!

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

Successfully merging this pull request may close these issues.

2 participants