fix(rsync,scp): remove file-type marks from "ls -F" properly #1398
Conversation
akinomyoga
force-pushed
the
scp-refactor-1-3
branch
from
3227068 to
2e21621
Compare
test/t/test_scp.py
Outdated
| assert_bash_exec(bash, "mkfifo '%s/local_path_1-pipe'" % tmpdir) | ||
| except Exception: | ||
| pytest.skip( | ||
| "The present system does not allow creating a named pipe." |
There was a problem hiding this comment.
I wonder if there's a valid setup that would allow creating these, but just doesn't have mkfifo installed. The end result would be the same, but the skip message would be misleading. Maybe include the stringified exception in it and generalize a bit, something like f"Could not create a named pipe: {ex}"?
Pre-approved with this addressed some way.
There was a problem hiding this comment.
I wonder if there's a valid setup that would allow creating these, but just doesn't have
mkfifoinstalled.
I anticipated the opposite situation, where mkfifo is installed, yet creating a named pipe is not allowed. For example, MSYS 1.0 doesn't support creating a named pipe:
$ mkfifo a.pipe
mkfifo: cannot create fifo `a.pipe': Function not implementedAlthough I haven't tried, I also wonder if we can create a named pipe inside a USB memory stick (whose filesystem is usually FAT32).
There was a problem hiding this comment.
With #1397 merged, is this in a draft state for some other reason, perhaps the above discussion?
There was a problem hiding this comment.
Sorry, I've simply been busy. I'll take a look again soon.
There was a problem hiding this comment.
I added a clarification in b3e01d6..bc07307.
akinomyoga
force-pushed
the
scp-refactor-1-3
branch
from
2e21621 to
b3e01d6
Compare
The type-classifier characters for named pipes (|) and sockets (=) were not properly removed. These are escaped by a backslash before removing, so the backslashes are left. In this patch, we remove the type-classifier characters before performing the backslash escaping.
akinomyoga
force-pushed
the
scp-refactor-1-3
branch
from
b3e01d6 to
bc07307
Compare
|
I've merged this. |
akinomyoga
changed the title
This was a part of #1371 but was separated. Waiting for #1397.
Details are described in the commit message.