Support on windows

[ranaur]

Issue Details

While running on a Windows machine, the program syncs subdirectories on the main directory in S3. For example:

Imagine a local directory with two local files, on in the main directory, and another in a subdirectory: { a.txt, dir/a.txt }

Instead of creating the directory in S3, it creates two files in the "root" directory names: "a.txt" and "dir\a.txt" (note the backslash)

I figured out what had happened: the os.path.join has a different behavior in Windows and POSIX (Linux, Mac) machines. In the former \ is the path eparator, and in the latter / is the separator.

I changed the os.path.join calls to posixpath.join in s3 and local client, that ensures the use of "/" as path separator, and created two functions, posix2local on local.py the client can translate between the systems.

Operating System (uname -a)

CYGWIN_NT-6.1 D-00884413 2.5.2(0.297/5/3) 2016-06-23 14:29 x86_64 Cygwin

s4 version (s4 version)

0.2.20

python version (python --version)

Python 3.7.0

Steps to reproduce the issue

Just sync a directory with a subdirectory in a windows machine.

DEBUG log output (run with s4 --log-level=DEBUG)

not necessary

[ranaur]

I wrote the fix, but I couldn't commit it (sorry I'm not very experienced with github). I forked the project at https://github.com/ranaur/S4 and commited in there.
The files changed for this issue and for #155 and added an option I needed to add to use (to ignore SSL errors when the environment variable PYTHONWARNINGS is set to "ignore:Unverified HTTPS request".

Sorry to the hassle, I just tried to help. :-) Great project!

[MichaelAquilina]

Hi there @ranaur I've opened the PR for you in #160

I'll take a look at it, review the code and see if we can merge this in :)

This project does not officially support windows because I dont have access to any machines that will allow me to test it. However I will happily accept changes that will improve its support.

[ranaur]

My use case if sync between an Windows machine and a Mac. I may try to sync in a linux machine (I have a spare Raspberry PI at home somewhere). If you allow, any bug I find in this configuration I'll post to you. Since the ".index" file is shared between the two operating systems, it must be written in the same way (using "/" instead "\" on diretories), and, on windows, translate it back to the "\" format. The solution I used was simple, but maybe not the best. I think the best option would be use pathlib (and not os.path.join anymore) in both s3.py and local.py, but that would demand an major rewrite on the code, so I changed the use in s3.py to posixpath.join and made the "translation" function in local.py, that works on both plataforms. At least the solution does not has "if windows then ...." :-) When I get some free time, I'll try to make the daemon works on windows if you want (I need to study how this cam be done. AFAIK windows does not support the inotify_simple module)

…

On Sun, Oct 28, 2018 at 11:44 AM Michael Aquilina ***@***.***> wrote: Hi there @ranaur <https://github.com/ranaur> I've opened the PR for you in #160 <#160> I'll take a look at it, review the code and see if we can merge this in :) This project does not *officially* support windows because I dont have access to any machines that will allow me to test it. However I will happily accept changes that will improve its support. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#156 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACANMw79jagLGNmaqWwOVBsPurM5efxuks5upcKxgaJpZM4X8-0r> .

[MichaelAquilina]

When I get some free time, I'll try to make the daemon works on windows if you want (I need to study how this cam be done. AFAIK windows does not support the inotify_simple module)

There must be an alternative on Windows to watching files so please feel free to give this a go :) Looking forward to seeing the PR!

With regards fixing the path issue - I'll try see if we can find a more elegant solution. Friends of mine have mentioned pathlib to me before so it might be a good option to try out.

So in its current state, s4 still does not work correctly on windows I assume? (I released a bug fix yesterday for when it crashes)

[ranaur]

After my patches it worked well. I synced on windows and I got the files on the Mac. I didn't tested a bigger directory, because, due to my initial testing, the free limit of PUT requests on S3 went out this month. In november I´ll make more "production tests" :-).

…

On Mon, Oct 29, 2018 at 10:54 AM Michael Aquilina ***@***.***> wrote: When I get some free time, I'll try to make the daemon works on windows if you want (I need to study how this cam be done. AFAIK windows does not support the inotify_simple module) There must be an alternative on Windows to watching files so please feel free to give this a go :) Looking forward to seeing the PR! With regards fixing the path issue - I'll try see if we can find a more elegant solution. Friends of mine have mentioned pathlib to me before so it might be a good option to try out. So in its current state, s4 still does not work correctly on windows I assume? (I released a bug fix yesterday for when it crashes) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#156 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACANM4rMJoNrvoHth8grXofXIJzr8sf2ks5upwizgaJpZM4X8-0r> .

[ranaur]

Just found another error on windows machines. Sometimes I get this error:

D:\Users\GUSCH@Arquivos\src\S4>poetry run s4 s
d:\cacert.pem
Syncing s4 [D:\Users\GUSCH\arquivos\s4/ <=> s3://ranaur/s4/]
Creating teste\README.md (s3://ranaur/s4/ => D:\Users\GUSCH\arquivos\s4/)
0.00B [00:00, ?B/s]An error occurred while trying to update teste\README.md:
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: (file-name) 'D:\Users\GUSCH\AppData\Local\Temp\tmpdoj8vzh2'
Nothing to update

Just figures out why. The exception handler on put method of local.py (around line 127) is removing the temporary file before closing it.

I'll send the patch through git, but basically I changed the exception handler to allow the "shameful" windows file locking mechanism. :-)

        shutil.move(temp_path, path)
    except Exception:
        os.close(fd)
        os.remove(temp_path)
        raise
    else:
        os.close(fd)

Should I open an issue just for this, or I pigback it here?