0.5.0 fails to run on Windows #179

Closed
opened 6 months ago by Ghost · 6 comments
Ghost commented 6 months ago

img2pdf 0.5.0 is broken on Windows, it fails with error:

ERROR:img2pdf:error: 'Namespace' object has no attribute 'pdfa'
Traceback (most recent call last):
  File "C:/Users/212545221/tools/img2pdf/img2pdfVE/lib/python3.11/site-packages/img2pdf.py", line 4488, in main
    pdfa=args.pdfa,
         ^^^^^^^^^
AttributeError: 'Namespace' object has no attribute 'pdfa'
(img2pdfVE)

I get the same error in 3 environments: installed by pip in native Python for Windows, installed by pip in MSYS2/MinGW64 and with your provided .exe package.

This seems to be caused by 7f0bf47ff3 src/img2pdf.py: reformat with black:
The 'pdfa' argument is not created at all on win32 platform

I don't know exactly why this option is not supported on Windows (and I don't use PDF/A support), maybe it should either just be left enabled but marked as not supported (maybe with a warning), or at least forced to None some way or another.

(and, by the way, thanks for this very useful utility !)

img2pdf 0.5.0 is broken on Windows, it fails with error: ``` ERROR:img2pdf:error: 'Namespace' object has no attribute 'pdfa' Traceback (most recent call last): File "C:/Users/212545221/tools/img2pdf/img2pdfVE/lib/python3.11/site-packages/img2pdf.py", line 4488, in main pdfa=args.pdfa, ^^^^^^^^^ AttributeError: 'Namespace' object has no attribute 'pdfa' (img2pdfVE) ``` I get the same error in 3 environments: installed by pip in native Python for Windows, installed by pip in MSYS2/MinGW64 and with your provided .exe package. This seems to be caused by [7f0bf47ff3](https://gitlab.mister-muffin.de/josch/img2pdf/commit/7f0bf47ff385dd8af587a0b8285e33aad03021fd) *src/img2pdf.py: reformat with black*: The 'pdfa' argument is not created at all on win32 platform I don't know exactly why this option is not supported on Windows (and I don't use PDF/A support), maybe it should either just be left enabled but marked as not supported (maybe with a warning), or at least forced to None some way or another. (and, by the way, thanks for this very useful utility !)
josch commented 6 months ago
Owner

Thank you for the investigation! May I highlight you for future issues with Windows? In most cases when people reported Windows issues, they did not know how to install img2pdf on their platform in different ways or how to investigate git commits. I do not own any Windows box myself so usually Windows issues remain unfixed as I'm unable to investigate them.

Your analysis was spot-on. I unintentionally added a change that I was not with yet finished without noticing it. If you have time, could you test if the following patch fixes the issue:

diff --git a/src/img2pdf.py b/src/img2pdf.py
index 036232b..f851d92 100755
--- a/src/img2pdf.py
+++ b/src/img2pdf.py
@@ -3814,14 +3814,31 @@ def gui():
     app.mainloop()
 
 
+def file_is_icc(fname):
+    with open(fname, "rb") as f:
+        data = f.read(40)
+    if len(data) < 40:
+        return False
+    return data[36:] == b"acsp"
+
+
+def validate_icc(fname):
+    if not file_is_icc(fname):
+        raise argparse.ArgumentTypeError('"%s" is not an ICC profile' % fname)
+    return fname
+
+
 def get_default_icc_profile():
     for profile in [
         "/usr/share/color/icc/sRGB.icc",
         "/usr/share/color/icc/OpenICC/sRGB.icc",
         "/usr/share/color/icc/colord/sRGB.icc",
     ]:
-        if os.path.exists(profile):
-            return profile
+        if not os.path.exists(profile):
+            continue
+        if not file_is_icc(profile):
+            continue
+        return profile
     return "/usr/share/color/icc/sRGB.icc"
 
 
@@ -4093,13 +4110,22 @@ RGB.""",
     )
 
     if sys.platform == "win32":
-        pass
+        # on Windows, there are no default paths to search for an ICC profile
+        # so make the argument required instead of optional
+        outargs.add_argument(
+            "--pdfa",
+            type=validate_icc,
+            help="Output a PDF/A-1b compliant document. The argument to this "
+            "option is the path to the ICC profile that will be embedded into "
+            "the resulting PDF.",
+        )
     else:
         outargs.add_argument(
             "--pdfa",
             nargs="?",
             const=get_default_icc_profile(),
             default=None,
+            type=validate_icc,
             help="Output a PDF/A-1b compliant document. By default, this will "
             "embed either /usr/share/color/icc/sRGB.icc, "
             "/usr/share/color/icc/OpenICC/sRGB.icc or "

Thanks!

Thank you for the investigation! May I highlight you for future issues with Windows? In most cases when people reported Windows issues, they did not know how to install img2pdf on their platform in different ways or how to investigate git commits. I do not own any Windows box myself so usually Windows issues remain unfixed as I'm unable to investigate them. Your analysis was spot-on. I unintentionally added a change that I was not with yet finished without noticing it. If you have time, could you test if the following patch fixes the issue: ```patch diff --git a/src/img2pdf.py b/src/img2pdf.py index 036232b..f851d92 100755 --- a/src/img2pdf.py +++ b/src/img2pdf.py @@ -3814,14 +3814,31 @@ def gui(): app.mainloop() +def file_is_icc(fname): + with open(fname, "rb") as f: + data = f.read(40) + if len(data) < 40: + return False + return data[36:] == b"acsp" + + +def validate_icc(fname): + if not file_is_icc(fname): + raise argparse.ArgumentTypeError('"%s" is not an ICC profile' % fname) + return fname + + def get_default_icc_profile(): for profile in [ "/usr/share/color/icc/sRGB.icc", "/usr/share/color/icc/OpenICC/sRGB.icc", "/usr/share/color/icc/colord/sRGB.icc", ]: - if os.path.exists(profile): - return profile + if not os.path.exists(profile): + continue + if not file_is_icc(profile): + continue + return profile return "/usr/share/color/icc/sRGB.icc" @@ -4093,13 +4110,22 @@ RGB.""", ) if sys.platform == "win32": - pass + # on Windows, there are no default paths to search for an ICC profile + # so make the argument required instead of optional + outargs.add_argument( + "--pdfa", + type=validate_icc, + help="Output a PDF/A-1b compliant document. The argument to this " + "option is the path to the ICC profile that will be embedded into " + "the resulting PDF.", + ) else: outargs.add_argument( "--pdfa", nargs="?", const=get_default_icc_profile(), default=None, + type=validate_icc, help="Output a PDF/A-1b compliant document. By default, this will " "embed either /usr/share/color/icc/sRGB.icc, " "/usr/share/color/icc/OpenICC/sRGB.icc or " ``` Thanks!

I am having the issue on Windows. What would be the pip3 command to apply the patch?

I am having the issue on Windows. What would be the pip3 command to apply the patch?
josch commented 6 months ago
Owner

I am having the issue on Windows. What would be the pip3 command to apply the patch?

You don't do git with pip. What you want to do, is clone this git repository, apply the patch with git apply or patch -p1 and then start src/img2pdf.py and see if it's fixed.

> I am having the issue on Windows. What would be the pip3 command to apply the patch? You don't do git with pip. What you want to do, is clone this git repository, apply the patch with `git apply` or `patch -p1` and then start `src/img2pdf.py` and see if it's fixed.

This could be because I am a rookie, but I am getting this error:

git apply patch.txt
warning: src/img2pdf.py has type 100644, expected 100755
error: patch failed: src/img2pdf.py:3814
error: src/img2pdf.py: patch does not apply

This could be because I am a rookie, but I am getting this error: git apply patch.txt warning: src/img2pdf.py has type 100644, expected 100755 error: patch failed: src/img2pdf.py:3814 error: src/img2pdf.py: patch does not apply
josch commented 6 months ago
Owner

So that you do not have to bother with patches I made this into a commit with a merge request: #180

I also pushed this to github so there is now a new exe on appveyor in case you do not to run it from git:

https://ci.appveyor.com/project/josch/img2pdf/builds/48465352/artifacts

So that you do not have to bother with patches I made this into a commit with a merge request: https://gitlab.mister-muffin.de/josch/img2pdf/pulls/180 I also pushed this to github so there is now a new exe on appveyor in case you do not to run it from git: https://ci.appveyor.com/project/josch/img2pdf/builds/48465352/artifacts

Thank you, this fixed the problem for me. :-)

Thank you, this fixed the problem for me. :-)
josch closed this issue 5 months ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: josch/img2pdf#179
Loading…
There is no content yet.