Accept a single file as a command line argument #54

[pombredanne]

This patch allow to run the scc CLI with a single file path as an argument
rather than a directory.
I explicitly license my contribution under a choice MIT or The Unlicense.

Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

[dbaggerman]

Snake case isn't very common in Go, isSoloFile would be more consistent with the rest of the codebase.

However, there is actually a simpler way to solve this. Simply replacing

all, _ := ioutil.ReadDir(root)

with

rootFileInfo, _ := os.Lstat(root)
if !rootFileInfo.IsDir() {
        root = filepath.Dir(root)
}
all := []os.FileInfo{rootFileInfo}

is the only change necessary in processor/file.go. That lets us better reuse the existing logic below for handling files vs directories.

Other than that, it looks good to me. Thanks for the PR.

[pombredanne]

Snake case isn't very common in Go, isSoloFile would be more consistent with the rest of the codebase.

My snakyness shows up right of the bat... Thank you for the much appreciated review

[pombredanne]

re

rootFileInfo, _ := os.Lstat(root)
if !rootFileInfo.IsDir() {
       root = filepath.Dir(root)
}
all := []os.FileInfo{rootFileInfo}

is the only change necessary in processor/file.go. That lets us better reuse the existing logic below for handling files vs directories.

That's much cleaner alright!

[pombredanne]

@dbaggerman Your alternate solution no longer works with directories.

[dbaggerman]

Oh, good catch. I tested ./scc . which did work, but other directories don't.

I realise now that the if !rootFileInfo.IsDir() condition shouldn't be there. Try

rootFileInfo, _ := os.Lstat(root)
// filepath.Clean will remove trailing folder slashes, which otherwise change the behaviour of filepath.Dir
root = filepath.Dir(filepath.Clean(root))
all := []os.FileInfo{rootFileInfo}

That works for all the cases I can think of. See if it you can find a way to break it.

[dbaggerman]

Actually, one issue with this is that ignoring the error on os.Lstat now causes a panic if an invalid file is given, instead of the old behaviour of just giving empty results. We'll have to catch that now and exit the goroutine gracefully.

[pombredanne]

Actually, one issue with this is that ignoring the error on os.Lstat now causes a panic if an invalid file is given, instead of the old behaviour of just giving empty results. We'll have to catch that now and exit the goroutine gracefully.

@dbaggerman I would have no idea where to start to do this... But there is another issue. Your approach works... BUT IMHO no longer walks in parallel as the input for a dir is now a dir and not its list of child directories.

[pombredanne]

Snaykyness removed and I am waiting for your feedback to my last point to make any further change

[dbaggerman]

Yeah, you're right about it not walking in parallel with my approach. It already isn't always properly parallel (for example when all the code is in one subdirectory), but I don't want to make the general case worse. Might be worth looking at redesigning that, but I won't hold up your PR.

I'll have one more look when I've got time, but I did notice an obscure error case that your new validation isn't catching:

./scc main.go/                     
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x54936c]

When a file exists, it passes your test but later gets processed as a directory which is invalid.

[pombredanne]

./scc main.go/
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x54936c]

@dbaggerman fixed in latest commit. Tell me if you want some more refactoring still!

[dbaggerman]

Ok, looks good to me now. Thanks @pombredanne.

[pombredanne]

@dbaggerman thanks ++ 🙇‍♂️