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

Should work with Solaris #31

Closed
rprikulis opened this issue May 8, 2019 · 24 comments
Closed

Should work with Solaris #31

rprikulis opened this issue May 8, 2019 · 24 comments

Comments

@rprikulis
Copy link

rprikulis commented May 8, 2019

Building on Solaris fails.

karrick/godirwalk/readdir.go:20:9: undefined: readdirents
karrick/godirwalk/readdir.go:46:9: undefined: readdirnames

Adding solaris to the build tags in readdir_unix.go results in:

karrick/godirwalk/readdir_unix.go:41:7: undefined: inoFromDirent
karrick/godirwalk/readdir_unix.go:55:13: de.Type undefined (type *syscall.Dirent has no field or method Type)
karrick/godirwalk/readdir_unix.go:56:9: undefined: syscall.DT_REG
@karrick
Copy link
Owner

karrick commented May 8, 2019

Do you have a Solaris system to test this on?

@karrick
Copy link
Owner

karrick commented May 8, 2019

Ah apparently you have. Sorry I used a mobile client to read the issue and failed to scroll the text field all the way to the right.

I’ll take a look at the syscall data structures for Solaris and see how one would add support. However I do not have a Solaris system to test on, so every release could possibly break Solaris functionality.

@karrick
Copy link
Owner

karrick commented May 8, 2019

Interestingly, the Solaris files for syscall do not have DT_REG and other directory entry type constants, and the syscall.Dirent structure for Solaris does not have a Type field.

type Dirent struct {
    Ino       uint64
    Off       int64
    Reclen    uint16
    Name      [1]int8
    Pad_cgo_0 [5]byte
}

@karrick
Copy link
Owner

karrick commented May 8, 2019

Which begs the question, then how does the standard library get the directory entry type? Well, it invokes Stat on each file. In other words, the standard library just reads the names from the directory entries for a directory, then comes back and stats each file to get the type. However, this second step is usually unnecessary because the type is already read and in memory when the file system directory entry is first parsed.

Well, apparently on most operating systems it is, but on Solaris it is not.

@karrick
Copy link
Owner

karrick commented May 8, 2019

@rprikulis, would you mind testing the solaris branch on your Solaris box in verbose mode and paste the results?

go test -v

@rprikulis
Copy link
Author

You mean

go test -v
# github.com/karrick/godirwalk [github.com/karrick/godirwalk.test]
./follow.go:15:20: undefined: evalSymlinksHelper
./withoutType.go:19:3: cannot use nil as type os.FileMode in return argument
note: module requires Go 1.12FAIL       github.com/karrick/godirwalk [build failed]

?

@rprikulis
Copy link
Author

rprikulis commented May 8, 2019

on go 1.12

go test -v
# github.com/karrick/godirwalk [github.com/karrick/godirwalk.test]
./follow.go:15:20: undefined: evalSymlinksHelper
./readdir.go:21:9: undefined: readdirents
./readdir.go:48:9: undefined: readdirnames
FAIL    github.com/karrick/godirwalk [build failed]

git checkout solaris
Branch 'solaris' set up to track remote branch 'solaris' from 'origin'.
Switched to a new branch 'solaris'

go test -v
# github.com/karrick/godirwalk [github.com/karrick/godirwalk.test]
./follow.go:15:20: undefined: evalSymlinksHelper
./withoutType.go:19:3: cannot use nil as type os.FileMode in return argument
FAIL    github.com/karrick/godirwalk [build failed]

@karrick karrick self-assigned this May 8, 2019
@karrick
Copy link
Owner

karrick commented May 8, 2019

It would be so much better were I to again have a Solaris box... I miss my OpenSolaris system, but that was about a decade ago that I retired it.

Thanks for the continued feedback. I’ll add the appropriate build tags for the additional files and push again.

@rprikulis
Copy link
Author

I can set up a Smartos zone for you to play with.

@karrick
Copy link
Owner

karrick commented May 8, 2019

Okay, I have updated the solaris branch again, hopefully fixing the build flags a little bit more durably.

Please give the test command another try.

@karrick
Copy link
Owner

karrick commented May 8, 2019

@rprikulis, that is very kind for you to offer, but I would rather avoid logging into other people's hosts because I do not want to bear the responsibility if anything goes wrong.

@rprikulis
Copy link
Author

Hmm. did I mess something up? Getting the same:

go test -v
# github.com/karrick/godirwalk [github.com/karrick/godirwalk.test]
./withoutType.go:19:3: cannot use nil as type os.FileMode in return argument
FAIL    github.com/karrick/godirwalk [build failed]

@karrick
Copy link
Owner

karrick commented May 8, 2019

No, that was my copy/paste mistake for handling that error when I refactored that bit of code out to handle the error.

Now I have re-learned to type GOOS=solaris go build before I commit to make sure it at least builds properly given the build tags.

Please pull and test again.

@rprikulis
Copy link
Author

Looks better or at least different:

go test -v
=== RUN   TestReadDirents
--- PASS: TestReadDirents (0.00s)
=== RUN   TestReadDirentsSymlinks
--- PASS: TestReadDirentsSymlinks (0.00s)
=== RUN   TestReadDirnames
--- PASS: TestReadDirnames (0.00s)
=== RUN   TestWalkSkipDir
=== RUN   TestWalkSkipDir/SkipFileAtRoot
=== RUN   TestWalkSkipDir/SkipFileUnderRoot
=== RUN   TestWalkSkipDir/SkipDirAtRoot
=== RUN   TestWalkSkipDir/SkipDirUnderRoot
=== RUN   TestWalkSkipDir/SkipDirOnSymlink
--- PASS: TestWalkSkipDir (0.00s)
    --- PASS: TestWalkSkipDir/SkipFileAtRoot (0.00s)
    --- PASS: TestWalkSkipDir/SkipFileUnderRoot (0.00s)
    --- PASS: TestWalkSkipDir/SkipDirAtRoot (0.00s)
    --- PASS: TestWalkSkipDir/SkipDirUnderRoot (0.00s)
    --- PASS: TestWalkSkipDir/SkipDirOnSymlink (0.00s)
=== RUN   TestWalkNoAccess
--- FAIL: TestWalkNoAccess (0.00s)
    walk_test.go:157:
        (GOT)
                []string(nil)
        (WNT)
                []string{"/tmp/godirwalk-332033600/dir6/noaccess"}
=== RUN   TestWalkFollowSymbolicLinksFalse
--- PASS: TestWalkFollowSymbolicLinksFalse (0.00s)
=== RUN   TestWalkFollowSymbolicLinksTrue
--- PASS: TestWalkFollowSymbolicLinksTrue (0.00s)
=== RUN   TestWalkSymbolicRelativeLinkChain
--- PASS: TestWalkSymbolicRelativeLinkChain (0.00s)
=== RUN   TestPostChildrenCallback
--- PASS: TestPostChildrenCallback (0.00s)
FAIL
exit status 1
FAIL    github.com/karrick/godirwalk    0.028s

@karrick
Copy link
Owner

karrick commented May 8, 2019

I am not sure why your most recent comments are showing up near the top of this conversation, and oddly enough, the time stamp for my previous comment is "karrick commented 7 hours from now", which is ludicrous.

@rprikulis
Copy link
Author

Lol.
I wondered why do my posts not show up anymore

@karrick
Copy link
Owner

karrick commented May 8, 2019

@rprikulis, your posts are showing up, but the times are out of order. It seems as if a handful of our earlier posts have a timestamp in the future.

Regardless, go ahead and re-run the tests. I do not expect it to pass, but when it fails, it will dump the file system rooted at the root of the test scaffolding directory so I can figure out what's going on with file permissions on Solaris.

@rprikulis
Copy link
Author

readdirents.out.txt

@karrick
Copy link
Owner

karrick commented May 8, 2019

I got the file list output; thanks. Looking into the cause of the test failure. The failing test is ensuring the Walk function does not visit entries that do not provide the user with permissions. The test output shows that this particular test directory does not give the user adequate permissions to visit that directory.

d--------- /dir6/noaccess

So now it's time for me to scratch my head a bit and think about this issue some more.

@karrick karrick changed the title Does not build on Solaris Should work with Solaris May 8, 2019
@rprikulis
Copy link
Author

Latest results:
godirwalk1.txt

If I think about it I have seen and wondered about this behavior that from some programs directories and files with no access are truly invisible. Is most annoying when you try and fail to delete a dir with such invisible content inside.

@karrick
Copy link
Owner

karrick commented May 9, 2019

I agree. The expectection is that this library provide the exact same results as the standard library, when this library is invoked with merely the same callback. So file system entries are visited or skipped in exactly the same way on a given platform.

Furthermore, this library does have the ability to deviate from standard library behavior when some options, such as FollowSymbolicLinks is set.

As a result, I have added a test to call filepath.Walk and godirwalk.Walk for the entire test root directory, and ensure the contents match precisely.

Then there will be additional tests to ensure that when various options, such as FollowSymbolicLinks is set, this library behave as advertised.

That is my plan, and I'll post when complete.

@karrick karrick mentioned this issue May 10, 2019
@karrick
Copy link
Owner

karrick commented May 10, 2019

@rprikulis, would you mind pulling the latest from master and running tests again.

If the tests do not pass, please paste output from go test -v.

@rprikulis
Copy link
Author

Thanks for the quick resolution!
Problem seems to be solved.

Testing the latest master gives:

=== RUN   TestReadDirents
--- PASS: TestReadDirents (0.00s)
=== RUN   TestReadDirentsSymlinks
--- PASS: TestReadDirentsSymlinks (0.00s)
=== RUN   TestReadDirnames
--- PASS: TestReadDirnames (0.00s)
=== RUN   TestWalkCompatibleWithFilepathWalk
=== RUN   TestWalkCompatibleWithFilepathWalk/test_root
--- PASS: TestWalkCompatibleWithFilepathWalk (0.00s)
    --- PASS: TestWalkCompatibleWithFilepathWalk/test_root (0.00s)
=== RUN   TestWalkSkipDir
=== RUN   TestWalkSkipDir/skip_file_at_root
=== RUN   TestWalkSkipDir/skip_dir_at_root
=== RUN   TestWalkSkipDir/skip_nodes_under_root
=== RUN   TestWalkSkipDir/SkipDirOnSymlink
--- PASS: TestWalkSkipDir (0.00s)
    --- PASS: TestWalkSkipDir/skip_file_at_root (0.00s)
    --- PASS: TestWalkSkipDir/skip_dir_at_root (0.00s)
    --- PASS: TestWalkSkipDir/skip_nodes_under_root (0.00s)
    --- PASS: TestWalkSkipDir/SkipDirOnSymlink (0.00s)
=== RUN   TestWalkFollowSymbolicLinks
--- PASS: TestWalkFollowSymbolicLinks (0.00s)
=== RUN   TestErrorCallback
=== RUN   TestErrorCallback/halt
=== RUN   TestErrorCallback/skipnode
--- PASS: TestErrorCallback (0.00s)
    --- PASS: TestErrorCallback/halt (0.00s)
    --- PASS: TestErrorCallback/skipnode (0.00s)
=== RUN   TestPostChildrenCallback
--- PASS: TestPostChildrenCallback (0.00s)
PASS
ok      github.com/karrick/godirwalk    0.031s

So I can close this issue.

@karrick
Copy link
Owner

karrick commented May 10, 2019

Thanks for running the tests again under Solaris.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants