-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add Capability function to query fs capabilities #59
Changes from all commits
a22d183
d198577
e2ca534
5bb98d7
c1e3d52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package billy_test | ||
|
||
import ( | ||
"testing" | ||
|
||
. "gopkg.in/src-d/go-billy.v4" | ||
"gopkg.in/src-d/go-billy.v4/test" | ||
|
||
. "gopkg.in/check.v1" | ||
) | ||
|
||
type FSSuite struct{} | ||
|
||
func Test(t *testing.T) { TestingT(t) } | ||
|
||
var _ = Suite(&FSSuite{}) | ||
|
||
func (s *FSSuite) TestCapabilities(c *C) { | ||
cases := []struct { | ||
caps Capability | ||
expected bool | ||
}{ | ||
{LockCapability, false}, | ||
{ReadCapability, true}, | ||
{ReadCapability | WriteCapability, true}, | ||
{ReadCapability | WriteCapability | ReadAndWriteCapability | TruncateCapability, true}, | ||
{ReadCapability | WriteCapability | ReadAndWriteCapability | TruncateCapability | LockCapability, false}, | ||
{TruncateCapability | LockCapability, false}, | ||
} | ||
|
||
// This filesystem supports all capabilities except for LockCapability | ||
fs := new(test.NoLockCapFs) | ||
|
||
for _, e := range cases { | ||
c.Assert(CapabilityCheck(fs, e.caps), Equals, e.expected) | ||
} | ||
|
||
dummy := new(test.BasicMock) | ||
c.Assert(Capabilities(dummy), Equals, DefaultCapabilities) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,6 +127,11 @@ func (fs *OS) Readlink(link string) (string, error) { | |
return os.Readlink(link) | ||
} | ||
|
||
// Capabilities implements the Capable interface. | ||
func (fs *OS) Capabilities() billy.Capability { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation of this should state that actual capabilities are dependent on the underlying filesystem. So, for example, if the filesystem is mounted as read-only, it cannot be written and it will return error on opening in write mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice that each filesystem could query these, that is, detect that the FS is mounted read only and do not return CapWrite. Either way the Capability was more about what was implemented in the billy filesystem than in the underlying OS fs. 👍 to explicitly tell this in the documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, adding some checking for |
||
return billy.DefaultCapabilities | ||
} | ||
|
||
// file is a wrapper for an os.File which adds support for file locking. | ||
type file struct { | ||
*os.File | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to maintain backwards compatibility, this method should assume a fixed set of capabilities for filesystems not implementing
Capable
, not something that can evolve over time.Maybe we could rename
CapAll
toCapDefault
and state explicitly in the documentation thatCapDefault
values will never change without a major version bump.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is silly and very personal :) but just adding 2 cents...
For me
Cap
sounds like Capacity (especially that we have in go global functioncap
which returns capacity). So maybe we can renameCapability
toAbility
(orFSMode
) and consts toReadable
,Writable
,Seekable
,Lockable
, ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something like this for the interfaces implemented by the FS. This implementation may not be very Goish. It tries to mimic open flags (
O_RDONLY | O_SYNC
) for operations that are not implemented in the billy FS even if the methods are there. One example is go-billy-siva. The files can not be opened in read and write mode. We can check with this flags before using one algorithm or the other:Other example is memory FS. The locking methods are no-ops.
Capability
may be changed toFeature
if it seems too close to capacity.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature sounds good to me. But I'm not totally against capability, but just abbreviation
cap
looks more like capacity.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
Capability
prefix instead ofCap
also looks more in line with our convention of avoiding abbreviations, specially if there is any ambiguity.