Fix std::remove on folders

[trzecieu]

std::remove expects EISDIR if it has been called on a directory: https://github.com/kripken/emscripten/blob/master/system/lib/libc/musl/src/stdio/remove.c

int remove(const char *path)
{
    int r = syscall(SYS_unlink, path);
    return (r && errno == EISDIR) ? syscall(SYS_rmdir, path) : r;
}

This fix removes mapping from EISDIR to EPERM on unlink, it has been added long time ago: #1388

CC:
@inolen
@kripken

---

I wasn't able about C++ test, I did my best.

[kripken]

I'm worried about a posix/linux difference here. Probably @inolen knows best about that.

@asria, do the tests pass? I would verify that the default and other suites pass.

[trzecieu]

Original change has been done according to: http://man7.org/linux/man-pages/man2/unlink.2.html like @inolen wrote. From the other side we use musl for standard lib implementation what explicitly says: "musl - an implementation of the standard library for Linux-based systems"

Going back in history I found a place where this line was added: a9fb60e
Unfortunately without brother explanation.

This is how I tested it:

• I used compiled 1.36.1, and in order to define a baseline state. I run:

./runner.py default   
./runner.py other skip:other.test_scons skip:other.test_llvm_lit

I had to remove test_scons due error:

ERROR: test_scons (test_other.other)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/trzeci/SDK/emsdk/emscripten/tag-1.36.1/tests/test_other.py", line 1998, in test_scons
    Popen(['scons']).communicate()
  File "/usr/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1340, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

• I cherrypicked from my changes only the test and applied onto 1.36.1
• I executed the same commands:

./runner.py default 
./runner.py other skip:other.test_scons skip:other.test_llvm_lit

It failed on the new test, what was expected.

• I cherrypicked changes from library_fs.js
• I executed the same command for tests

It found 1 error:

• ERROR: test_unistd_unlink (test_core.default)

https://github.com/kripken/emscripten/blob/master/tests/unistd/unlink.c#L82-L86
What looks like an expected result.

It's a little bit complicated test flow, but I wasn't able to hook my fork to SDK in order to use it.

Possible ways to solve it:

• Change the test with a patch:

diff --git a/tests/unistd/unlink.c b/tests/unistd/unlink.c
index 2e51214..ca81828 100644
--- a/tests/unistd/unlink.c
+++ b/tests/unistd/unlink.c
@@ -79,7 +79,7 @@ void test() {

   err = unlink("dir-readonly");
   assert(err == -1);
-#ifdef __linux__
+#if defined(__linux__) || defined(__EMSCRIPTEN__)
   assert(errno == EISDIR);
 #else
   assert(errno == EPERM);

• Change implementation of remove.c. (What I'm not big fan of)

Do you you have any ideas or recommendations?

[kripken]

Thank you for the detailed investigation!

Ok, if I understand correctly, this does in fact look like a linux/posix difference. And therefore musl does what it does (as a linux libc), and also therefore if we change that unistd test to make it handle emscripten like linux, then it would work.

That sounds good to me. In general when in doubt, we've done what linux does.

[kripken]

please leave a comment here, that if we were POSIX compliant we would do [..], but we instead do the same thing that linux does.

[trzecieu]

Done, I added a proper comment to unlink.c.

[kripken]

Great, thanks! Squashed and merged to incoming.