Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3 #4

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Python3 #4

wants to merge 17 commits into from

Conversation

carlilek
Copy link

msrsync did not work with Python 3.5.2 without modifications, so I copied it to msrsync3 and made some light changes to the code. I have not fully tested it, but with a relatively simple msrsync3 -p32 --progress --stat -r '--inplace -W' /path/to/src/ /path/to/dest/ it seems to work.

--Ken

@jbd
Copy link
Owner

jbd commented Dec 31, 2017

Thank you for this pull request. Give me a few days and I'll get back to you.

@carlilek
Copy link
Author

carlilek commented Dec 31, 2017 via email

@jbd
Copy link
Owner

jbd commented Dec 31, 2017

Just wanted to get my hands on that sweet sweet scandir optimization, and thought I'd share.

Note that you can have the scandir module installed for python2. msrync will use it if available.

@carlilek
Copy link
Author

carlilek commented Jan 1, 2018

Ran into unicode problems. Around line 659, where I changed from wb to w. Clearly something I'm missing here.

@jbd
Copy link
Owner

jbd commented Jan 1, 2018

Are you able to reproduce the problem ?

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@jbd
Copy link
Owner

jbd commented Jan 2, 2018

My latest attempt (running now) is: 659 bfile.write((entry + '\0').encode('utf-8','surrogateescape'))

I'm not sure that's a good idea. I'm not sure how rsync --files-from will handle that. Be careful.
The best would be to identified the pathname that cause problem.

Could you give the initial exception when you were using "wb" flags ?

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@jbd
Copy link
Owner

jbd commented Jan 2, 2018

Thank you for the feedback.

I'm hugely interested into this edge case, I'd like to perfectly understand what is going on. If you can provide a path that reproduce the problem that would be ideal.

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@jbd
Copy link
Owner

jbd commented Jan 2, 2018

Can you run this command inside the directory containing your file and give me the output ?

find . -maxdepth 1 -type f -name "mEOS_0s8_Int_45*zip" -exec sh -c 'printf "%-10s %s\n" "$1" "$(printf "$1" | xxd -pu )"' None {} \;

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@jbd
Copy link
Owner

jbd commented Jan 2, 2018

Perfect, thank you. I'll try to get back to you in the next few days.

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@carlilek
Copy link
Author

carlilek commented Jan 2, 2018 via email

@jbd
Copy link
Owner

jbd commented Jan 2, 2018

That's a bug. I'll correct that as soon as possible. Thank you for testing !

carlilek and others added 4 commits January 9, 2018 11:29
@rayrrr
Copy link

rayrrr commented Aug 26, 2018

FWIW I just had to copy about 1TB of data from a Debian 9 server to an external USB2 HDD as quickly as possible, and this PR's branch in its current form did a great job at maxing out the USB2 bandwidth, with no bugs or drawbacks.

Speed: 28.6 M/s
Rsync workers: 4
Total rsync's processes (161) cumulative runtime: 113237.4s
Crawl time: 0.2s (0.0% of total runtime)
Total time: 28636.7s

So that literally got me to done in about 1/4 the time it otherwise would have taken. Thank you both, @jbd and @carlilek!

@carlilek
Copy link
Author

carlilek commented Aug 26, 2018 via email

@jbd
Copy link
Owner

jbd commented Aug 26, 2018

Hello,

thank you for the kind words. It reminds me about @carlilek's work and this windows encoding problem I should clearly address. I really hope having some time to figure it out.

@carlilek
Copy link
Author

carlilek commented Aug 26, 2018 via email

@carlilek
Copy link
Author

I need to complete some more testing, but I think this is the working version, where it will handle utf-8 and windows-1252 encoding, albeit possibly with a slowdown due to inefficient bucketing

--Ken

updating to upstream msrsync to incorporate fixes into msrsync3
bringing msrsync up to date so I can incorporate fixes into msrsync3
@kklem0
Copy link

kklem0 commented Aug 20, 2019

Hi,

This should become a blocker soon as Python 2 is dying fast.
I would propose to use path.py and let the pro do the pro's work, path.py handles encoding pretty well and already solved a few corner cases, so either dig into their code or just import it.
Version 11 still supports Python 2 and there's no new features/bugfixes in the latest version.

https://pypi.org/project/path.py

They do however also mention that with Python 3.6 everything should be solved, but that might be too big of a jump for now? https://www.python.org/dev/peps/pep-0519/

@kklem0
Copy link

kklem0 commented Aug 20, 2019

A quick look into path.py I think they're using sys.getdefaultencoding(), although the older version used sys.getfilesystemencoding()

@Gunni
Copy link

Gunni commented Oct 17, 2019

pathlib was also introduced in Python 3.4, maybe it can handle these issues better?

@serge2016
Copy link

Hello! Any progress here?

@jbd
Copy link
Owner

jbd commented Apr 29, 2020

No progress, sorry for that.

@yellowsoar yellowsoar mentioned this pull request Jun 10, 2020
@HaleTom
Copy link

HaleTom commented Jan 1, 2021

I'd love to be able to use this tool on my system which is only python 3.

Any 2021 new year resolutions affecting this by any chance?

@carlilek
Copy link
Author

carlilek commented Jan 1, 2021 via email

@jbd
Copy link
Owner

jbd commented Jan 12, 2021

Any 2021 new year resolutions affecting this by any chance?

Hello,

I understand the frustration around python3, but there has been no progress. I'd like to do a proper "review" soon of carlilek patches before merging. It looks like you could already use carlilek's work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants