Validate pfx/dosdevices symlink after copy

[shymega]

This PR adds validation for the special dosdevices folder on the Proton prefix.

[stackrot]

Heyo @shymega

Just had chance to go over this, couple notes:

Spotted that #251 was closed w/o merging but this PR mentions needing it merged first, so just need to sort below

• pfx/dosdevices is still in the ignore list, so the directory never gets copied
• The validation at wemod.py:567 is gated by if os.path.isdir(dosdevices), which will be False since the dir was skipped
• No os.makedirs() call exists to create it

Smaller things:

• assert gets stripped by python -O (wemod.py:578-579) so those would silently disappear. We don't use -O, but worth keeping in mind
• create_symlink will silently do nothing if dst exists as a regular file rather than a symlink, the if os.path.islink(dst) guard just skips it with no error, probs not a big deal since they should always be sym's, but worth noting

[shymega]

@stackrot I think it'd be easier for me to see which parts you mean if you use the Code Review feature GitHub offers. We can then converse on threads on the Git hunks you select in the diff, and that'll work much better :-)

[stackrot]

Oh cool, didn't know that was a thing (I interact with github mostly through neovim haha)
It looks like you fixed the main issues I flagged.

I've left a comment on the other one (if I did it right), but it's very minor and probably won't come up.

[shymega]

Oh cool, didn't know that was a thing (I interact with github mostly through neovim haha) It looks like you fixed the main issues I flagged.

I've left a comment on the other one (if I did it right), but it's very minor and probably won't come up.

Yeah, I did have a project I was working on to convert GitHub's UI to a NNTP based interface, so you could comment on PRs with email. Never got very farwith it.

I've fixed your review.

As for your other comments:

assert gets stripped by python -O (wemod.py:578-579) so those would silently disappear. We don't use -O, but worth keeping in mind

Yeah, we don't use that option, but yes, fair enough. The assert is more of a final check.

create_symlink will silently do nothing if dst exists as a regular file rather than a symlink, the if os.path.islink(dst) guard just skips it with no error, probs not a big deal since they should always be sym's, but worth noting

We could make it remove the file and replace it with a symlink, but I felt that was too destructive.

[marvin1099]

@shymega did you test this then, if so then seems fine to merge.

• after the "Yeeted from..." comment is removed.

[shymega]

@shymega did you test this then, if so then seems fine to merge.

* after the "Yeeted from..." comment is removed.

The comment is to provide attribution to the snippet of Python I copied from a developer's site. I think we should keep it, but I can change the 'Yeeted from' to 'Taken from'

[marvin1099]

@shymega did you test this then, if so then seems fine to merge.

* after the "Yeeted from..." comment is removed.

The comment is to provide attribution to the snippet of Python I copied from a developer's site. I think we should keep it, but I can change the 'Yeeted from' to 'Taken from'

We can easly come up with a version like this, whiout knowing the code infact let me do it right now,

import os
import pathlib
import shutil
from typing import Union

def safe_symlink(src: Union[str, pathlib.Path],
                 dst: Union[str, pathlib.Path],
                 *, replace: bool = True) -> None:
    """
    Create a symlink from *src* to *dst*, handling existing files,
    directories, or incorrect symlinks.

    Parameters
    ----------
    src : path‑like
        Existing file or directory that should become the link target.
    dst : path‑like
        Desired location of the symlink.
    replace : bool, default True
        If True, conflicting ``dst`` objects are removed before linking.
        If False, a conflict raises ``OSError``.
    """
    src_path = pathlib.Path(src).resolve()
    dst_path = pathlib.Path(dst)

    if not src_path.exists():
        raise FileNotFoundError(f"Source does not exist: {src_path}")

    # Handle existing destination
    if dst_path.lexists():                     # true for files, dirs, or symlinks
        if dst_path.is_symlink():
            # Correct symlink – nothing to do
            if dst_path.resolve() == src_path:
                return
            # Wrong target
            if not replace:
                raise OSError(f"Symlink {dst_path} points to {dst_path.resolve()}, "
                              f"expected {src_path}")
            dst_path.unlink()                  # remove incorrect link
        else:
            # File or directory
            if not replace:
                raise OSError(f"{dst_path} exists and is not a symlink")
            if dst_path.is_dir():
                shutil.rmtree(dst_path)        # delete whole directory tree
            else:
                dst_path.unlink()              # delete regular file

    # Create link
    os.symlink(src_path, dst_path)

There we go independent and in dosn't even replace correct links.

[shymega]

I suppose we could use that. Seems a bit overly complex for our needs, but I'll integrate it when I get a moment.

[marvin1099]

wemod.py:578-579

We should probably not use an assert if we wanna ship a binary then this check should not be skipped.
We can just test and exit with error / log as usual.

Also the z drive is not a necessity, i don't think we need to check that one.
The z drive is mapped to "linux /" on some wine setups, but not all have and it works without it, so i don't think is needed, am I wrong?

Also for the create_symlink changes a made a merge request (#257).
But feel free to manually add it it you don't wanna marge like that.

[shymega]

On 30.04.2026 05:03, Marvin1099 wrote: marvin1099 left a comment (DeckCheatz/wemod-launcher#252) > [wemod.py:578-579](https://github.com/DeckCheatz/wemod-launcher/blob/285c2b28505055a8719ec9b7920e5beacbec3a73/src/wemod.py#L578-L579) We should probably not use an assert if we wanna ship a binary then this check should not be skipped.

But we don't ship a binary, so I see no harm in keeping an assert. I think having an assert reassures from a logic standpoint.

We can just test and exit with error / log as usual.

Yes, we're doing that already currently, not sure if needs anymore work.

Also the z drive is not a necessity, i don't think we need to check that one. The z drive is mapped to "linux /" on some wine setups, but not all have and it works without it, so i don't think is needed, am I wrong?

Hm... we don't want unexpected behaviour, but equally I know of no Proton implementation that /doesn't/ map Z: to "/". Do you have examples?

Also for the create_symlink changes a made a merge request (#257).

Thanks, I'll review this weekend.

…

But feel free to manually add it it you don't wanna marge like that. -- Reply to this email directly or view it on GitHub: #252 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[marvin1099]

But we don't ship a binary, so I see no harm in keeping an assert.

Just wanted to future proof.
Asserts are meant for checks that dont need to be checked after compile is used.
So since this is a always needs completion check the assert is offplace.
Thats what o think anyway.

Hm... we don't want unexpected behaviour, but equally I know of no
Proton implementation that /doesn't/ map Z: to "/". Do you have
examples?

Just regular wine.
I actually prefer using wemod without proton.
With lutris, wine is way faster then proton.
In steam you can't use wine but as soon as you go outside of steam using wine directly works well.
Plus I dont think any code relies on drive z to be there so is it not unneeded.

[shymega]

On 01.05.2026 00:27, Marvin1099 wrote: marvin1099 left a comment (DeckCheatz/wemod-launcher#252) > But we don't ship a binary, so I see no harm in keeping an assert. Just wanted to future proof. Asserts are meant for checks that dont need to be checked after compile is used. So since this is a always needs completion check the assert is offplace. Thats what o think anyway.

Ah, fair.

> Hm... we don't want unexpected behaviour, but equally I know of no >Proton implementation that /doesn't/ map Z: to "/". Do you have >examples? Just regular wine. I actually prefer using wemod without proton. With lutris, wine is way faster then proton. In steam you can't use wine but as soon as you go outside of steam using wine directly works well.

I'm surprised at this. I've never seen Wine not map Z: to the host's root filesystem. Can you provide a screenshot of `DIR Z:` from a Wine shell?

…

-- Reply to this email directly or view it on GitHub: #252 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[marvin1099]

I'm surprised at this. I've never seen Wine not map Z: to the host's root filesystem. Can you provide a screenshot of DIR Z: from a Wine shell?

Must be in older wine versions.
The newest one seems to map Z:/ to root.
I do remember wine not doing this in older versions.
I usually saw the wrappers (like bottles) make this Z:/ drive.
But seems that this is in fact now default, not sure since when.
I usualy creata wine prefixes like this "WINEPREFIX=/new/path wine hostname"
and that didn't create a Z:/ mapping in older versions.
Since that seems to be included now that check would be fine.
But the ting is i did not see any coude of ours using "Z:/".
Why add that check if we dont use "Z:/", I'm i wrong?

[shymega]

I'm surprised at this. I've never seen Wine not map Z: to the host's root filesystem. Can you provide a screenshot of DIR Z: from a Wine shell?

Must be in older wine versions. The newest one seems to map Z:/ to root. I do remember wine not doing this in older versions. I usually saw the wrappers (like bottles) make this Z:/ drive. But seems that this is in fact now default, not sure since when. I usualy creata wine prefixes like this "WINEPREFIX=/new/path wine hostname" and that didn't create a Z:/ mapping in older versions. Since that seems to be included now that check would be fine. But the ting is i did not see any coude of ours using "Z:/". Why add that check if we dont use "Z:/", I'm i wrong?

I'm almost certain Wine has always mapped Z: to /. I don't think wrappers have done that for it. I remember using Wine back in 2008, and it did it then too.

Are you thinking of the z: link in dosdevices? I wonder if we're thinking of two different things.

I think we need the Z: check, as we call wand.bat from Z: - it's not in the C: drive.

[marvin1099]

I think we need the Z: check, as we call wand.bat from Z: - it's not in the C: drive.

True, completly missed that one
Never mind then.