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

Issue reading PO files on windows. #757

Closed
Nigel2392 opened this issue Jan 4, 2024 · 3 comments
Closed

Issue reading PO files on windows. #757

Nigel2392 opened this issue Jan 4, 2024 · 3 comments

Comments

@Nigel2392
Copy link
Contributor

Nigel2392 commented Jan 4, 2024

PO files cannot properly be opened on windows. This is because the file gets locked.

You might receive permission errors like so:

PermissionError at /admin/localize/translate/59/pofile/upload/
[Errno 13] Permission denied: 'C:\\Users\\<NOT_THE_RIGHT_USERPROFILE_FOLDER????>\\AppData\\Local\\Temp\\tmp6o550iu9'

Where NOT_THE_RIGHT_USERPROFILE_FOLDER would be NIGELV~1 when it should be NigelvanGoodAd.

This is due to an issue in opening the tempfile in view/edit_translation.upload_pofile. The temporary file should be opened with delete=False, and then properly unlinked.

The fixed code should be:


@require_POST
def upload_pofile(request, translation_id):
    translation = get_object_or_404(Translation, id=translation_id)

    instance = translation.get_target_instance()
    if not user_can_edit_instance(request.user, instance):
        raise PermissionDenied

    do_import = True

    # Set delete to false. This fixes some windows errors when creating tempfiles.
    # This is due to the Windows OS locking temporary files when open.
    with tempfile.NamedTemporaryFile(delete=False) as f:
        # Note: polib.pofile accepts either a filename or contents. We cannot pass the
        # contents directly into polib.pofile or users could upload a file containing
        # a filename and this will be read by polib!
        f.write(request.FILES["file"].read())
        f.flush()

    # Move indentation back, we should open the file outside of the with statement.
    # Delete must be set to false, otherwise the file will be deleted before we can open it.
    try:
        po = polib.pofile(f.name)

    except (OSError, UnicodeDecodeError):
       # Annoyingly, POLib uses OSError for parser exceptions...
       messages.error(request, _("Please upload a valid PO file."))
       do_import = False

    if do_import:
        translation_id = po.metadata["X-WagtailLocalize-TranslationID"]
        if translation_id != str(translation.uuid):
            messages.error(
                request,
                _(
                    "Cannot import PO file that was created for a different translation."
                ),
            )
            do_import = False

    if do_import:
        translation.import_po(po, user=request.user, tool_name="PO File")
        messages.success(request, _("Successfully imported translations from PO File."))

    # Delete the created tempfile
    try:
        os.unlink(f.name)
    except OSError:
        pass

    # Work out where to redirect to
    next_url = get_valid_next_url_from_request(request)
    if not next_url:
        # Note: You should always provide a next URL when using this view!
        next_url = reverse("wagtailadmin_home")

    return redirect(next_url)

@zerolab
Copy link
Collaborator

zerolab commented Jan 4, 2024

@Nigel2392 this is like a side effect of none of the localize developers using Windows. Happy to review a PR if you're up for submitting one

@Nigel2392
Copy link
Contributor Author

@Nigel2392 this is like a side effect of none of the localize developers using Windows. Happy to review a PR if you're up for submitting one

I don't really understand github and their pull request system, sorry... I've monkeypatched it in my own project, so it won't bother me when developing. Respectfully - I'm only here to bring it to attention. I'm planning to put some time into git's pull system in the future; I'll get back to it then.

@zerolab
Copy link
Collaborator

zerolab commented Jan 5, 2024

Understood. And thank you again for raising these issues. It is most helpful to get reports from as many real-world users as possible.

zerolab pushed a commit that referenced this issue Feb 25, 2024
* Resolve case insensitivity issues
* Resolve windows PO imports permissionerrors
zerolab pushed a commit that referenced this issue Feb 25, 2024
* Resolve case insensitivity issues
* Resolve windows PO imports permissionerrors
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