Bug in commit 8a7223ed4b89567f5e763a1245b85c61df8263bf

[dmoklaf]

Under MacOS X, Golang 1.6, the commit 8a7223ed4b89567f5e763a1245b85c61df8263bf broke the tool:

when using copyrighter with a package name postfixed with "/..." (so, not with a valid filesystem path), the tool fails

It appears the bug comes from os.Stat: the error returned with a path ending with "/..." is not catched by os.IsNotExist(err). The error string sent back by os.Stat is "stat xxx/...: not a directory"

[jmhodges]

Could you give me a full example?

I wrote a test that seems to pass my understanding of this ticket. It's in this PR. https://github.com/jmhodges/copyfighter/pull/10

The important bit is the string argument to check. Is that what you meant?

[jmhodges]

Hm, it was passing locally. Fixing it up.

[jmhodges]

yeah, that test doesn't pass even before that commit, though?

[dmoklaf]

What I can say is that your tool worked before, called exactly in the same manner. I had integrated copyfighter into a shell script (the shell equivalent of metalinter) and it performed fine for a few months at least.

My script line for copyfighter call is

$GOPATH/bin/copyfighter -max 96 $TARGET/… | grep -v $"_ffjson.go" || :

where $TARGET is the name of the package that I want to check, along with all its children.

If you remove the clutter, that is equivalent to

copyfighter mypackage/…

I can’t tell (yet) why it worked with your previous code, I didn’t analyze it. If you need assistance to investigate/reproduce it with the old code, let me know, I can prepare something for you.

With the new code, the reason is, it seems, a quirck in the golang os lib (at least on MacOSX) : when stat-ing a file whose path ends with « /… », instead of sending back an error that this file does not exist (which your new code assumes), the os lib sends back an error that this file "is not a directory" (?). That leaves 2 alternatives for decision, if you want your tool to support /… package patterns: 1- We consider this is a bug in the os lib -> we create a ticket for the golang project 2- We consider this quirk from the os lib has some logic, it is the responsibility of the caller (copyfighter) to handle exotic errors coming from an exotic path (it's true that a path ending with ... is not so common). In such case the patch in your code is simple, we need to add a test that the path ends with « /… ». If it does, we dont stat the path (as it is exotic), we directly jump to the code handling package search.