search

julian-klode / dir2ogg

Official repository of dir2ogg
https://jak-linux.org/projects/dir2ogg/
GNU General Public License v2.0
15 stars 6 forks source link

Use shutil.move rather than os.rename #4

Open je-vv opened 7 years ago

je-vv commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Yeah, that's what I just wrote ;)