Open chylex opened 1 year ago
chylex
commented
1 year ago It looks like this breaks the functionality of --download-archive, it hasn't added any new entries since I started using the --paths parameter to set a different temp folder.
bashonly
commented
1 year ago Where is the rest of the log? Please do not remove debug header info
OSError: [Errno 18] Invalid cross-device link is not the error being thrown, it is caught and being handled, during which the actual error PermissionError: [Errno 1] Operation not permitted is thrown.
shutil.move is used to move the file from temp dir to final dir, because as the code comment notes: "os.rename cannot move between volumes"
But shutil.move first tries os.rename, and catches the OSError (the above-mentioned Errno 18), and then retries with platform-specific methods meant to handle moving files between different filesystems.
The fallback method tries to copy the file to the destination directory and then remove it from the source directory in separate steps. After the file itself is copied, then it tries to copy over the file's permissions and metadata, and it is on this line that it throws the PermissionError:
lookup("utime")(dst, ns=(st.st_atime_ns, st.st_mtime_ns),I did some digging into this, and it appears that this is due to a bug/limitation in WSL1: https://github.com/microsoft/WSL/issues/5742
Unfortunately, the WSL developers have opted not to fix this in WSL1, and have closed the issue as fixed-in-wsl2
You could try running yt-dlp with sudo to see if that works around the issue. Or else you may need to upgrade to WSL2
dirkf
commented
1 year ago --paths allows what have always been simple file moves to become potentially lengthy copy operations, but let's assume that, as implied by the code comment, this is an intended feature.
Python documentation, echoed by the code comment, states that shutil.move() should be used rather than os.rename() to avoid an error when the destination isn't on the same filesystem, but SO commenters suggest that this can also fail, and this shows one failure mode, apparently a Python library bug. The source says:
A lot more could be done here... A look at a mv.c shows a lot of the issues this implementation glosses over.
The sequence here is that the file is moved by copying it, then copying some file metadata, and finally deleting the original. As step 2 fails, the original file isn't deleted. Stricter, but perhaps less useful, error handling would have erased the copied file rather than leaving the operation half-complete.
A hint as to how this should be fixed is that the _copytree() routine actually catches the specific error
try:
copystat(src, dst)
except OSError as why:
# Copying file access times may fail on Windows
if getattr(why, 'winerror', None) is None:
errors.append((src, dst, str(why)))whereas the copy2() routine that just copies a single file doesn't, because as the specification of shutil.copystat() notes:
...
copystat()never returns failure.
If the above were true (and assuming, since the function doesn't return anything, that "returns failure" means "raises any Exception"), the error reported here could not happen.
chylex
commented
1 year ago Where is the rest of the log? Please do not remove debug header info
It's there, but for some reason it's formatted on one line.
It looks ok in GitHub's editor, not sure what's going on. Tried copy/pasting the log again but it did the exact same thing.
EDIT: I ran the command again and piped the log to a file instead of copying it from tmux, it looks ok now. Out of interest, this is the original debug header with the weird scrolling:
[debug] Command-line config: ['-vU', '--paths', 'home:/mnt/x/yt-dlp', '--paths', 'temp:/tmp/yt-dlp', 'https://www.youtube.com/watch?v=BaW_jenozKc'] [debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8 [debug] yt-dlp version stable@2023.07.06 [b532a3481] (zip) [debug] Python 3.11.2 (CPython x86_64 64bit) - Linux-4.4.0-19041-Microsoft-x86_64-with-glibc2.36 (OpenSSL 3.0.9 30 May 2023, glibc 2.36) [debug] exe versions: ffmpeg 6.0-static (setts), ffprobe 6.0-static [debug] Optional libraries: mutagen-1.46.0, sqlite3-2.6.0 [debug] Proxy map: {} [debug] Loaded 1855 extractors [debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest Available version: stable@2023.07.06, Current version: stable@2023.07.06 Current Build Hash: cedf44a9cf3c241520a692c1006d276e6f434d4d945727c47cb1a70d32953f4a yt-dlp is up to date (stable@2023.07.06) You could try running yt-dlp with sudo to see if that works around the issue.
That does appear to work around the issue, thanks for the tip. Since this seems to be an issue with Python's error handling, do you think it'd be an acceptable solution to catch this error when shutil.move is called, and fallback to a simple copy+delete? I could make a PR for it.
dirkf
commented
1 year ago Does the compat mechanism support replacing shutil.copystat() with a version wrapped in with contextlib.suppress(Exception): such that the wrapped version is used by shutil.copy2()? That would make the module behaviour match the doc/spec:
... On platforms where some or all of this functionality is unavailable,
copy2()will preserve all the metadata it can;copy2()never raises an exception because it cannot preserve file metadata.
copy2()usescopystat()to copy the file metadata. ...
DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE
Checklist
Provide a description that is worded well enough to be understood
Hi!
I have a WSL1 instance connected to an SMB server. The SMB server is the
X:drive on Windows, and then mounted to/mnt/xviasudo mount -t drvfs X: /mnt/x.Running the following command in a WSL1 terminal:
produces the mentioned error when moving the file from temp to home. The file is successfully copied to home, but it never gets deleted from temp.
Provide verbose output that clearly demonstrates the problem
yt-dlp -vU <your command line>)'verbose': TruetoYoutubeDLparams instead[debug] Command-line config) and insert it belowComplete Verbose Output