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

os: RemoveAll fails on Hammer2 #33403

Closed
hxw opened this issue Aug 1, 2019 · 4 comments
Closed

os: RemoveAll fails on Hammer2 #33403

hxw opened this issue Aug 1, 2019 · 4 comments

Comments

@hxw
Copy link

@hxw hxw commented Aug 1, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.7 dragonfly/amd64

Does this issue reproduce with the latest release?

1.12 is the latest

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hsw/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="dragonfly"
GOOS="dragonfly"
GOPATH="/home/hsw/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/dragonfly_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build675907830=/tmp/go-build -gno-record-gcc-switches"

What did you do?

In shell  on a DragonFly BSD Hammer2 partition create a non-empty directory:
  mkdir -p data/sub
  touch data/sub/file

Sample program to remove the directory::

package main

import(
        "fmt"
        "os"
)

func main() {
        err := os.RemoveAll("data")
        if nil != err {
                fmt.Printf("ra error: %s\n", err)
        }
}

What did you expect to see?

no output (as happens with UFS, MSDOS and Hammer1 fielsystems)

What did you see instead?

removal fails on Hammer2 with the following message:
ra error: directory not empty

suggested fix

Add the ENOTEMPTY to the condition in os/removeall_at.go

--- removeall_at.go.orig        2019-07-09 05:29:25.000000000 +0800
+++ removeall_at.go     2019-08-01 15:02:09.743625000 +0800
@@ -70,7 +70,7 @@
        // the parent directory, but this entry might still be a directory
        // whose contents need to be removed.
        // Otherwise just return the error.
-       if err != syscall.EISDIR && err != syscall.EPERM && err != syscall.EACCES {
+       if err != syscall.EISDIR && err != syscall.EPERM && err != syscall.EACCES && err != syscall.ENOTEMPTY {
                return &PathError{"unlinkat", base, err}
        }
@ALTree ALTree added this to the Go1.14 milestone Aug 1, 2019
@ALTree ALTree changed the title os.RemoveAll fails on Hammer2 os: RemoveAll fails on Hammer2 Aug 1, 2019
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 2, 2019

I don't know what Hammer2 is.

The Open Group spec for unlinkat (https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/functions/unlink.html) seems pretty clear that unlinkat without the AT_REMOVEDIR flag should return EPERM. The notes say that a program should also handle EISDIR. There is no provision for ENOTEMPTY.

Is it possible to file this as a bug against Hammer2? It is returning ENOTEMPTY when it should be returning EPERM.

@hxw

This comment has been minimized.

Copy link
Author

@hxw hxw commented Aug 2, 2019

on DragonFly BSD various fs return different messages

  • MSDOS and UFS return EPERM
  • Hammer1 returns EIDIR
  • Hammer2 returns ENOTEMPTY

I will discuss on their IRC channel

@hxw

This comment has been minimized.

Copy link
Author

@hxw hxw commented Aug 5, 2019

I just tested a change that Mat Dillon did to the Hammer2 file system, so that it returns EISDIR to be comaptable with Hammer2. The change is in: DragonFlyBSD/DragonFlyBSD@f261d7b

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 5, 2019

Thanks, it sounds like this will be fixed outside of the Go repo, so closing this issue.

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.