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

Fix 'go generate ./...' on windows #3708

Closed
wants to merge 1 commit into from

Conversation

part22
Copy link
Contributor

@part22 part22 commented Jul 1, 2024

When running the go generate ./... command, pkg\cheatsheet\generate.go panics because the embedded translation files cannot be found. Previously mentionned here.

This is caused by the method filepath.Join, which uses the OS-specific path separator.

Join joins any number of path elements into a single path, separating them with an OS-specific Separator.

See filepath.Join documentation.


On Windows, it is \ (backslash). However, embed.FS.ReadFile expects the path separator to be / (slash) regardless of the OS.

The path separator is a forward slash, even on Windows systems.

See embed documentation.


To fulfill the contract of embed.FS.ReadFile, the path.Join method should be used instead since it always uses slashes as the path separator.

Join joins any number of path elements into a single path, separating them with slashes.

See path.Join documentation.


I have tried running go generate ./... on Ubuntu using WSL and the functionality seems unchanged.

Note
I also ran in another problem coming from pkg\jsonschema\generate.go where it panics because the string it is looking for terminates by \n.

DocumentationCommentStart    = "<!-- START CONFIG YAML: AUTOMATICALLY GENERATED with `go generate ./..., DO NOT UPDATE MANUALLY -->\n"

However on Windows the new line characters are \r\n.
The solution was to disable git autocrlf (git config core.autocrlf false) and to renomalize the files (git add --renormalize .) to have the new line character match the one in DocumentationCommentStart.
It might be worth including something in the doc about how to run go generate ./....

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

When running the 'go generate ./...' command, pkg\cheatsheet\generate.go
panics because the embedded translation files cannot be found.

This is caused by the method 'filepath.Join', which uses the OS-specific
path separator.

┌ filepath.Join Documentation ─────────────────────────────────────────┐
│                                                                      │
│ Join joins any number of path elements into a single path,           │
│ separating them with an OS specific Separator.                       │
│                                                                      │
└────────────────────── https://pkg.go.dev/path/[email protected]#Join ┘

On Windows, it is '\' (backslash). However, 'embed.FS.ReadFile' expects
the path separator to be '/' (slash) regardless of the OS.

┌ embed Documentation ─────────────────────────────────────────────────┐
│                                                                      │
│ The path separator is a forward slash, even on Windows systems.      │
│                                                                      │
└──────────────────────────────────────────── https://pkg.go.dev/embed ┘

To fulfill the contract of 'embed.FS.ReadFile', the 'path.Join' method
should be used instead since it always uses slashes as the path
separator.

┌ path.Join Documenation ──────────────────────────────────────────────┐
│                                                                      │
│ Join joins any number of path elements into a single path,           │
│ separating them with slashes.                                        │
│                                                                      │
└─────────────────────────────── https://pkg.go.dev/[email protected]#Join
@stefanhaller
Copy link
Collaborator

Thanks, but this is already fixed by #3705 and #3706 (which I told you about here 😉).

@part22
Copy link
Contributor Author

part22 commented Jul 1, 2024

Haha, yeah, I saw it too late.
I'll close the PR.

@part22 part22 closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants