Skip to content

Commit

Permalink
upspinfs: corrects link size handling
Browse files Browse the repository at this point in the history
Correctly reports the symlink size as the size of the content
printed out by Readlink.

Old code reported size 0 for symlinks.  This is correct per
upspin specification, but not correct when upspin is viewed
through a filesystem mount.  Reporting incorrect size led
to weird artifacts when using git on top of upspinfs, as
git would interpret file size of 0 as file content removal,
and would generate spurious diffs.

Fixes #619.

Change-Id: I17d3ebd04998f7167d060cb90256936a75cc8d7f
Reviewed-on: https://upspin-review.googlesource.com/c/upspin/+/19280
Reviewed-by: David Presotto <presotto@gmail.com>
  • Loading branch information
filmil authored and presotto committed Apr 3, 2019
1 parent 15046f4 commit e10ca8f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 8 deletions.
29 changes: 25 additions & 4 deletions cmd/upspinfs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,10 @@ func (n *node) openDir(context gContext.Context, req *fuse.OpenRequest, resp *fu
continue
}
cn.Lock()
if sz, err := child.Size(); err == nil {
cn.attr.Size = uint64(sz)
if sz, err := lstatSize(child, n); err == nil {
cn.attr.Size = sz
} else {
return nil, e2e(errors.E(op, err, n.uname))
}
cn.attr.Mtime = child.Time.Go()
cn.Unlock()
Expand Down Expand Up @@ -568,6 +570,25 @@ func (n *node) Remove(context gContext.Context, req *fuse.RemoveRequest) error {
return nil
}

// lstatSize returns a lstat-compatible size for the dir entry. The size only
// differs for symlinks. Upspin's DirEntry size for a link is zero, but for
// lstat, the size of the link is the size of the link content.
func lstatSize(de *upspin.DirEntry, n *node) (uint64, error) {
if !de.IsLink() {
s, err := de.Size()
return uint64(s), err
}
// It seems that upspin treats all symlinks that don't leave the filesystem
// as relative, so replicate that approach here. This may have interesting
// side effects for programs that care about precise link content, like
// version control systems.
p, err := n.upspinPathToHostPath(de.Link)
if err != nil {
return 0, err
}
return uint64(len(p)), nil
}

// Lookup implements fs.NodeStringLookuper.Lookup. 'n' must be a directory.
// We do not use cached knowledge of 'n's contents.
func (n *node) Lookup(context gContext.Context, name string) (fs.Node, error) {
Expand Down Expand Up @@ -616,12 +637,12 @@ func (n *node) Lookup(context gContext.Context, name string) (fs.Node, error) {
if de.IsLink() {
mode |= os.ModeSymlink
}
size, err := de.Size()
size, err := lstatSize(de, n)
if err != nil {
f.removeMapping(uname)
return nil, e2e(errors.E(op, n.uname, err))
}
nn := n.f.allocNode(n, name, mode, uint64(size), de.Time.Go())
nn := n.f.allocNode(n, name, mode, size, de.Time.Go())
if de.IsLink() {
nn.link = upspin.PathName(de.Link)
}
Expand Down
14 changes: 14 additions & 0 deletions cmd/upspinfs/upspinfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,13 @@ func testSymlink(t *testing.T, link, rooted, relative string, contents []byte) {
if val != relative {
fatalf(t, "%s: Readlink returned %s, expected %s:]", link, val, relative)
}
s, err := os.Lstat(link)
if err != nil {
fatal(t, err)
}
if s.Size() != int64(len(relative)) {
fatalf(t, "%s(%v): Lstat returned size %v, expected %v, relative: %q, rooted: %q:]", link, len(link), s.Size(), len(relative), relative, rooted)
}
remove(t, link)

// Create and test using relative name.
Expand All @@ -478,6 +485,13 @@ func testSymlink(t *testing.T, link, rooted, relative string, contents []byte) {
if val != relative {
fatalf(t, "%s: Readlink returned %s, expected %s", link, val, relative)
}
s, err = os.Lstat(link)
if err != nil {
fatal(t, err)
}
if s.Size() != int64(len(relative)) {
fatalf(t, "%s(%v): Lstat returned size %v, expected %v, relative: %q, rooted: %q:]", link, len(link), s.Size(), len(relative), relative, rooted)
}
}

// TestRename tests renaming a file.
Expand Down
8 changes: 4 additions & 4 deletions cmd/upspinfs/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ func (w *watchedRoots) refresh(n *node) error {
if de.IsLink() {
mode |= os.ModeSymlink
}
size, err := de.Size()
size, err := lstatSize(de, n)
if err != nil {
n.f.removeMapping(n.uname)
return e2e(errors.E(op, err))
}
n.attr.Size = uint64(size)
n.attr.Size = size
n.attr.Mode = mode
if de.IsLink() {
n.link = upspin.PathName(de.Link)
Expand Down Expand Up @@ -400,9 +400,9 @@ func (d *watchedRoot) handleEvent(e *upspin.Event) error {
if e.Entry.IsLink() {
mode |= os.ModeSymlink
}
size, err := e.Entry.Size()
size, err := lstatSize(e.Entry, n)
if err == nil {
n.attr.Size = uint64(size)
n.attr.Size = size
} else {
log.Debug.Printf("upspinfs.watch: %s", err)
}
Expand Down

0 comments on commit e10ca8f

Please sign in to comment.