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

cmd/go: go fmt does not ignore dot files #18383

Closed
XANi opened this issue Dec 20, 2016 · 12 comments
Closed

cmd/go: go fmt does not ignore dot files #18383

XANi opened this issue Dec 20, 2016 · 12 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@XANi
Copy link

XANi commented Dec 20, 2016

Documentation specifies that gofmt ignores dotfiles but go fmt will still pass them as arguments to gofmt. For example (Go 1.7.3)

-> ᛯ echo package main >main.go
-> ᛯ strace -f gofmt . 2>&1 |grep nope
[pid  7607] lstat(".nope.go", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
-> ᛯ strace -f go fmt 2>&1 |grep nope
[pid  8013] lstat("/tmp/asd/.nope.go", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
[pid  8029] execve("/usr/lib/go-1.7/bin/gofmt", ["/usr/lib/go-1.7/bin/gofmt", "-l", "-w", ".nope.go", "main.go"], [/* 64 vars */] <unfinished ...>
[pid  8029] stat(".nope.go",  <unfinished ...>
[pid  8029] openat(AT_FDCWD, ".nope.go", O_RDONLY|O_CLOEXEC <unfinished ...>
[pid  8029] read(3, "package nope\n", 512) = 13

some editors like emacs create symliks as locks for modified files like that: .#wifitoken.go -> xani@ghroth.11692:1482232478 and that causes go fmt to spew errors:

-> ᛯ go fmt
stat .#wifitoken.go: no such file or directory
exit status 2

which could also be avoided if go fmt correctly ignored files

@ianlancetaylor ianlancetaylor changed the title go fmt does not ignore same files as gofmt cmd/go: go fmt does not ignore dot files Dec 20, 2016
@ianlancetaylor
Copy link
Contributor

I'm not sure there is a bug here. Both programs are following their documentation. I think the open question is whether we should modify the go tool to ignore filenames that start with '.'.

@XANi
Copy link
Author

XANi commented Dec 20, 2016

I'd be nice to have consistency, for example go vet ignores dotfiles (even tho doc doesn't explicitly say anything about it), as does go build

@robpike
Copy link
Contributor

robpike commented Dec 20, 2016

Dot files are a Unix convention and not a universal one. In general tools should not ignore them; they are just files like any other.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 20, 2016

@robpike, there's no need for us to be stubborn about it, though. Many of our public talks on Go extol the virtues of convention over configuration (the GOPATH layout, file naming, etc). We should accept popular existing conventions (dot files), even if we don't all totally agree with them.

But consistency between "go fmt" and "gofmt" is probably a bigger argument.

@bradfitz bradfitz added this to the Go1.9 milestone Dec 20, 2016
@robpike
Copy link
Contributor

robpike commented Dec 20, 2016

But @bradfitz, dot files are only special on Unix. The convention is not universal and Go binaries do not run only on Unix systems.

Your point about consistency is of course valid, but consistent with what, and where?

@rakyll
Copy link
Contributor

rakyll commented Dec 20, 2016

Can't we just consistently ignore the hidden files depending on the system the tool is running? Dot files on Unix, hidden file attribute on Windows. There are many support files editors generate and it is terrible having to deal with them.

@ianlancetaylor
Copy link
Contributor

Wait, something is wrong here. The go tool, for better or worse, already ignores files whose name begins with '.' (and '_').

@ianlancetaylor
Copy link
Contributor

Oh, I see. go fmt specifically runs gofmt on all Go files that it finds, which includes the files that the go/build package stores in IgnoredGoFiles. The go/build package puts all non-matching files whose name ends in ".go" in IgnoredGoFiles. Files that start with '_' or '.' do not match, and therefore always wind up in IgnoredGoFiles. This affects go fmt, go fix, and go get -fix.

So now I think the bug is that the go tool should really ignore files that start with '_' or '.' for those three cases.

@XANi
Copy link
Author

XANi commented Dec 21, 2016

@rakyll you pretty much have to have same ignore list between platforms or else you will end up with inconsistencies with build process.

If say .something.go is ignored on unix (because dotfiles) but is not ignored on windows (because it does not have hidden attribute) then you will have different result on both platforms. And filename is pretty much only thing common between platforms so using any extra filesystem attributes could potentially add to the problem

@rakyll
Copy link
Contributor

rakyll commented Jan 3, 2017

@XANi I don't think we shouldn't care other than consistently ignoring dotfiles everywhere because dotfiles are portable. If you put your source in a hidden file/temp file, you are already inviting trouble because meaning of a hidden/temp file is dependent to your platform. The go tool should consider those files as invalid sources of code and ignore them all together.

The go tool already uses different ignore lists via build tags. I don't think our users are having a problem with keeping the integrity of their packages. I assume ignoring hidden files is not going to create more trouble.

@XANi
Copy link
Author

XANi commented Jan 3, 2017

@rakyll I didn't say it should care, just that relying on anything other than file name generates problems and IMO isn't even worth the effort of writing platform-specific code.

It is extra code that doesn't really help anyone, as you said if someone is using hidden attribute they probably will screw themselves at some point anyway, whether go tools ignore it or not.

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 8, 2017
@gopherbot
Copy link

CL https://golang.org/cl/45156 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants