Skip to content

Use a local Walk function #421

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Oct 29, 2019
Merged

Use a local Walk function #421

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
94e3d7b
Use a local Walk function
d-a-v Sep 22, 2019
7186d73
tabs
d-a-v Sep 22, 2019
23f0204
style
d-a-v Sep 22, 2019
fad3aa5
comment
d-a-v Sep 22, 2019
7403a39
retrigger CI
d-a-v Sep 22, 2019
9c82aad
retrigger CI
d-a-v Sep 23, 2019
aac3d81
Merge branch 'master' of github.com:arduino/arduino-cli
d-a-v Sep 23, 2019
38c71e9
style
d-a-v Sep 23, 2019
5503c56
check sketch for being a file
d-a-v Sep 23, 2019
75ffd4f
check sketch is not a directory
d-a-v Sep 23, 2019
98bc2e3
Merge branch 'master' of github.com:arduino/arduino-cli
d-a-v Oct 23, 2019
05576c9
additions:
d-a-v Oct 23, 2019
fe4cd8b
fixes from "task check"
d-a-v Oct 23, 2019
b51893a
updates per review
d-a-v Oct 28, 2019
cde4bec
style
d-a-v Oct 29, 2019
d41fa12
Update arduino/builder/sketch.go
d-a-v Oct 29, 2019
9de3344
fixes
d-a-v Oct 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixes from "task check"
  • Loading branch information
@d-a-v
d-a-v committed Oct 23, 2019
commit fe4cd8b6ac01ca005e32baf6ed59e693ab41773a
14 changes: 7 additions & 7 deletions arduino/builder/sketch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ package builder

import (
"bytes"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"regexp"
"strings"
"fmt"

"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/arduino/sketch"
Expand Down Expand Up @@ -59,8 +59,7 @@ func SketchSaveItemCpp(item *sketch.Item, destPath string) error {
return nil
}

// SimpleLocalWalk locally replaces filepath.Walk and/but goes through symlinks
func SimpleLocalWalkRecursive(root string, maxDepth int, walkFn func(path string, info os.FileInfo, err error) error) error {
func simpleLocalWalkRecursive(root string, maxDepth int, walkFn func(path string, info os.FileInfo, err error) error) error {

info, err := os.Stat(root)

Expand All @@ -75,13 +74,13 @@ func SimpleLocalWalkRecursive(root string, maxDepth int, walkFn func(path string

if info.IsDir() {
if maxDepth <= 0 {
return walkFn(root, info, errors.New("Filesystem bottom is too deep (directory recursion or filesystem really deep): " + root))
return walkFn(root, info, errors.New("Filesystem bottom is too deep (directory recursion or filesystem really deep): "+root))
}
maxDepth--
files, err := ioutil.ReadDir(root)
if err == nil {
for _, file := range files {
err = SimpleLocalWalkRecursive(root+string(os.PathSeparator)+file.Name(), maxDepth, walkFn)
err = simpleLocalWalkRecursive(root+string(os.PathSeparator)+file.Name(), maxDepth, walkFn)
if err == filepath.SkipDir {
return nil
}
Expand All @@ -92,9 +91,10 @@ func SimpleLocalWalkRecursive(root string, maxDepth int, walkFn func(path string
return nil
}

// SimpleLocalWalk locally replaces filepath.Walk and/but goes through symlinks
func SimpleLocalWalk(root string, walkFn func(path string, info os.FileInfo, err error) error) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simpleLocalWalkRecursive() is currently used in this module only, There's no need to wrap it and export it. Can you please remove the wrapper and use directly simpleLocalWalkRecursive()?
Regarding the maxDepth param, can you please create a private constant in this module (named for example maxFileSystemDepth) documenting it properly writing something like: As currently implemented on Linux, the maximum number of symbolic links that will be followed while resolving a pathname is 40? This way you can use something more meaningful as a parameter when calling the function.
Thanks!

// see discussion in https://github.com/arduino/arduino-cli/pull/421
return SimpleLocalWalkRecursive(root, 40, walkFn)
return simpleLocalWalkRecursive(root, 40, walkFn)
}

// SketchLoad collects all the files composing a sketch.
Expand Down Expand Up @@ -137,7 +137,7 @@ func SketchLoad(sketchPath, buildPath string) (*sketch.Sketch, error) {

if err != nil {
fmt.Printf("\nerror: %+v\n\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use the /cli/feedback package to print error messages like

feedback.Errorf("Error ....: %v", err)

return filepath.SkipDir;
return filepath.SkipDir
}

// ignore hidden files and skip hidden directories
Expand Down