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

Use shutil.move rather than os.rename #4

Open
je-vv opened this issue Apr 15, 2017 · 3 comments
Open

Use shutil.move rather than os.rename #4

je-vv opened this issue Apr 15, 2017 · 3 comments

Comments

@je-vv
Copy link

je-vv commented Apr 15, 2017

Whe the current directory FS (disk/partition) where dir2ogg is called from differs from the destination directory FS (disk/partition) of final files, then dir2ogg will fail like:

File "/usr/bin/dir2ogg", line 398, in decode
os.rename(tempwav, self.songwav)
OSError: [Errno 18] Invalid cross-device link

The following patch prevents such failure:

diff -Naur fastoggenc-old/fastoggenc fastoggenc/fastoggenc
--- fastoggenc-old/fastoggenc 2017-04-15 15:29:34.400646642 -0600
+++ fastoggenc/fastoggenc 2017-04-15 15:30:28.471085075 -0600
@@ -39,6 +39,7 @@
import sys
import gettext
import os, os.path
+import shutil
import re
import multiprocessing
import threading
@@ -546,7 +547,7 @@
if self.decoder == 'mplayer':
# Move the file for mplayer (which uses tempwav), so it works
# for --preserve-wav.
- os.rename(tempwav, self.songwav)
+ shutil.move(tempwav, self.songwav)
if retcode != 0:
return (False, None)
else:

I imagine there's another way around the issue, and it's making sure the temporal files are generated in the final destination rather than current directory, but in the end using shutil.move is a good easy solution, :-) See:

http://pythoncentral.io/how-to-rename-move-a-file-in-python

@julian-klode
Copy link
Owner

rename() is actually correct, the file is just at the wrong place. tempwav should really be created in the same directory as self.songwav. Copying it seems a bit awkward.

Or well, it could probably also just write to self.songwav directly. Not sure why there's the indirection.

@je-vv
Copy link
Author

je-vv commented Apr 15, 2017

Hmm, rename is OK, if dir2ogg was called always from the same directory, or at least same partition where the origin and destination files are...

While running dir2ogg, and generating wavs, I noticed the wavs are generated in the current directory where dir2ogg is called. Not on the same directory where the original wma/... file was, neither the same directory where the final ogg will be.

That's the reason os.rename fails, when the current directory (from where dir2ogg is called) filesystem differs from the original/destination files directory. In my case, current directory from where dir2ogg is called is a different partition (actually different disk, a SSD), than the partition where the directory to convert resides (different disk, a HD).

Under current way of dir2ogg, it seems os.rename will always fail, no matter full path attributes are passed, and that's why shutil.move looks like a better solution afterwords...

The non so obvious fix (I didn't look for it) would be to generate the temp wavs on the same directory the origin/destination files are. Therefore os.rename wouldn't have a problem with that...

@julian-klode
Copy link
Owner

Yeah, that's what I just wrote ;)

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

No branches or pull requests

2 participants