Windows path improvements

[ranaur]

Beware: long text. Grab a cofee first.

I tested and I found some little problems:

First the pip command didn't work. I cloned from your repository with -b option

git clone https://github.com/MichaelAquilina/S4/ -b windows_path_improvements

I hope I did it right.

Running with "my" version of the windows path fix

D:\Users\GUSCH\@Arquivos\mysrc\S4>poetry run s4 t
s4: [D:\Users\GUSCH\arquivos\s4 <=> s3://ranaur/s4]

D:\Users\GUSCH\@Arquivos\mysrc\S4>poetry run s4 s
Syncing s4 [D:\/Users/GUSCH/arquivos/s4/ <=> s3://ranaur/s4/]
Nothing to update

Running with your version:

D:\Users\GUSCH\@Arquivos\mysrc\test\S4>poetry run s4 t
s4: [D:\Users\GUSCH\arquivos\s4 <=> s3://ranaur/s4]

D:\Users\GUSCH\@Arquivos\mysrc\test\S4>poetry run s4 s
Syncing s4 [D:\Users\GUSCH\arquivos\s4/ <=> s3://ranaur/s4]

Conflict for "Novo Documento de Texto.txt". Which version would you like to keep
?
   (1) D:\Users\GUSCH\arquivos\s4/Novo Documento de Texto.txt updated at 2018-11
-16 19:00:12 (NOCHANGES)
   (2) s3://ranaur/s4Novo Documento de Texto.txt updated at None (CREATED)
   (d) View difference (requires the diff command)
   (X) Skip this file

Choice (default=skip): Quitting due to Keyboard Interrupt...

Note that the (1) file didn't join evenly "arquivos\s4/Novo Documento"
And (2) did't show the '/' between the files.
Also, the "Sync message" also has a path problem "Syncing s4 [D:\Users\GUSCH\arquivos\s4/ <=> s3://ranaur/s4]"

Then just merged your branch with my repository (I hope I did it right, it took some time) and pulled the changes below on s3.py and local.py files.

In s3.py, we should really use posixpath module instead os os.path. From https://docs.python.org/3/library/os.path.html:

"Note: Since different operating systems have different path name conventions, there are several versions of this module in the standard library. The os.path module is always the path module suitable for the operating system Python is running on, and therefore usable for local paths. However, you can also import and use the individual modules if you want to manipulate a path that is always in one of the different formats. They all have the same interface:"

Since S3 uses URI path conventions (uses / to separate directories), we should always use posixpath to handle it. I did the changes in s3.py and pulled it to the repository. (Commit 17bd1c5)

Now about local.py ...

First about windows file locking. In windows, by default, when you open a file the file gets "pinned": you can't rename, delete or move it. So, even when using temporary files, you always need to close them before renaming. I need to fix the "put" method try to handle it nicely. (commit 8d285c4)

I tried to sync a new file "teste/teste.txt". Looking inside the .index, I noticed it writes as teste\teste.txt. If I sync between windows and Linux machines I'll probably mess everything. IMHO the best way to handle this is always write the path in the ".index" file using "/" as separator.

D:\Users\GUSCH@Arquivos\mysrc\S4>gzip -d < index
{"Novo Documento de Texto.txt": {"remote_timestamp": 1542394812, "local_timestam
p": 1542397238.669858}, "as": {"remote_timestamp": 1542394813, "local_timestamp"
: 1542397239.092058}, "teste/No\u00e1o D\u00e7cum\u00c1nto de Texto.txt": {"remo
te_timestamp": 1542394813, "local_timestamp": 1542397239.530858}, "Novo Document
o de Texto (2).txt": {"remote_timestamp": 1542399900, "local_timestamp": null},
"teste 2/Novo Documento de Texto.txt": {"remote_timestamp": 1542397328, "local_t
imestamp": null}, "..pub": {"remote_timestamp": 1542397426, "local_timestamp": n
ull}, "Novo(a) Documento do Microsoft Publisher.pub": {"remote_timestamp": 15423
97426, "local_timestamp": 1542397426.638658}, "teste 2/Novo Documento de Texto (
2).txt": {"remote_timestamp": 1542399933, "local_timestamp": null}, "teste 2/tes
te.txt": {"remote_timestamp": 1542399933, "local_timestamp": null}, "teste\No\u
00e1o D\u00e7cum\u00c1nto de Texto.txt": {"remote_timestamp": 1542397239, "local
_timestamp": 1542397239.530858}, "teste\teste.txt": {"remote_timestamp": 154289
7416, "local_timestamp": 1542897416.8116808}}

(second-to-last line. The other files were created by my version os local.py)

Well. The fix for this is a little more tough. First we'll need to ensure that traverse() function always generates the index dict using posix convention. If we don't do this, when syncing between unix and windows we'll get strange results. (belive me, it happened when I was making my version).

Then we need to ensure that every time the local.py goes to the filesystem, it should use the local convention (\ or /), even when the file index uses the "/" format. Then, we need to ensure that the rest of the program shows the filenames using the "local convention". That's why I needed to make so many changes.

But first review the other changes. If we agree with them, I can pull the request to change the local.py to fix that..

Thanks for the time!

[MichaelAquilina]

Sorry @ranaur I've been on holiday for the past few days. Seems like from what you are explaining the bug is quite simple to fix.

I'm afraid you still havent grasped how to open a PR though as it still contains a messy history 😅

did you mean to reply to the original PR? #171

[MichaelAquilina]

Also the pip install command did not work because it does not seem to play well with poetry :( Might consider changing to pipenv or back to plain pip because it seems to be complicating the workflow rather than helping it.

[ranaur]

If you look at the differences, it is a small thing: two files an half of a dozen lines os insertions and delete. Why in the hell it says 730 commits, I really don't know. I hadn't done 730 commits. I swear! :-)

…

On Mon, Nov 26, 2018 at 10:29 AM Michael Aquilina ***@***.***> wrote: Also the pip install command did not work because it does not seem to play well with poetry :( Might consider changing to pipenv or back to plain pip because it seems to be complicating the workflow rather than helping it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#174 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACANMw--kE8GgkEKPqicBwD4V4Ky3V7vks5uy966gaJpZM4YvrpC> .

[ranaur]

I deleted my repository, and created it from scratch. I'll make anothe PR. I'll close this PR and make another one from scratch.

[ranaur]

Just made another PR. Here we go again. On Mon, Nov 26, 2018 at 5:48 PM Gustavo Ranaur Schoenaker <[email protected]> wrote:

…

If you look at the differences, it is a small thing: two files an half of a dozen lines os insertions and delete. Why in the hell it says 730 commits, I really don't know. I hadn't done 730 commits. I swear! :-) On Mon, Nov 26, 2018 at 10:29 AM Michael Aquilina < ***@***.***> wrote: > Also the pip install command did not work because it does not seem to > play well with poetry :( Might consider changing to pipenv or back to > plain pip because it seems to be complicating the workflow rather than > helping it. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#174 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ACANMw--kE8GgkEKPqicBwD4V4Ky3V7vks5uy966gaJpZM4YvrpC> > . >