Even after removing an icon, os.Remove won’t delete the folder, also no matter what It’s a bug to not be able to remove a folder just because of an icon and people should not have to do manual actions to delete a folder.
It will be resolved when the patch I made on the Go repo will make it in a public release even if it’s not any time If you want I can make a temporary patch for Syncthing while Go has not updated
I’m not certain your patch will do anything.
We already modify permissions before trying to delete things.
If your claim was generally true, syncthing would never be able to delete any directory.
More than three days spent proving that there is an issue, I went as far as proposing a solution, identifying exactly where in the code is the issue and patching the os lib.
I know it does because I did a unit test to check with or without patch and yes it has an impact, it’s amazing to have to go this far when I posted the screenshot just so that person under it says it is permission related when obviously it is not just looking at the screenshot 2 centimers over the reply button.
Would be ideal if we could programmatically create a dir with an icon. Then we can just have a unit test on it. Generally maybe just show the steps and outcome once with and once without the change. Then it’ll be clear.
If you want to speed things up for syncthing, please PR with that unit test.
Just to close the debate over the fact that an issue exists 
package main
import (
"fmt"
"io/ioutil"
"os"
"syscall"
)
func main() {
path := "./folder_with_icon"
fmt.Println(count_files(path))
err := os.Remove(path + "/desktop.ini")
if err != nil {
panic(err)
}
fmt.Println(count_files(path)) // works fine up to here
err = os.Remove(path)
if err != nil {
panic(err)
}
}
func count_files(path string) int {
files, _ := ioutil.ReadDir(path)
return len(files)
}
func Remove(name string) error {
p, e := syscall.UTF16PtrFromString(name)
if e != nil {
return &os.PathError{Op: "remove", Path: name, Err: e}
}
// Go file interface forces us to know whether
// name is a file or directory. Try both.
e = syscall.DeleteFile(p)
if e == nil {
return nil
}
e1 := syscall.RemoveDirectory(p)
if e1 == nil {
return nil
} else {
//Empty directories with "read-only" attribute on Windows can cause issues so we have to try with chmod to remove it
a, e2 := syscall.GetFileAttributes(p)
if e2 == nil && a&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 && os.IsPermission(e1) {
if fs, err := os.Stat(name); err == nil {
if err = os.Chmod(name, os.FileMode(0200|int(fs.Mode()))); err == nil {
e1 = syscall.RemoveDirectory(p)
}
}
}
}
// Both failed: figure out which error to return.
if e1 != e {
a, e2 := syscall.GetFileAttributes(p)
if e2 != nil {
e = e2
} else {
if a&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
e = e1
} else if a&syscall.FILE_ATTRIBUTE_READONLY != 0 {
if e1 = syscall.SetFileAttributes(p, a&^syscall.FILE_ATTRIBUTE_READONLY); e1 == nil {
if e = syscall.DeleteFile(p); e == nil {
return nil
}
}
}
}
}
return &os.PathError{Op: "remove", Path: name, Err: e}
}
Create a folder with an icon next to this code, try to delete with os, look at it fail.
Now look at it with the function patched under it and it works.
Pretty sure an automated test can be done for that, I will do a PR for it
Edit : Just checked and saw @calmh implementing the solution and opening a PR to make the temporary fix so no need to debate anymore on that
Do you still want a unit test for that ?
This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.