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

htmlmin fires warning in python 3.11 and is not maintained anymore #25

Closed
ldeluigi opened this issue Dec 5, 2022 · 20 comments
Closed

Comments

@ldeluigi
Copy link

ldeluigi commented Dec 5, 2022

INFO     -  DeprecationWarning: 'cgi' is deprecated and slated for removal in Python 3.13
  File "/usr/local/lib/python3.11/warnings.py", line 514, in _deprecated
    warn(msg, DeprecationWarning, stacklevel=3)
  File "/usr/local/lib/python3.11/site-packages/htmlmin/main.py", line 28, in
    import cgi

The original issue: mankyd/htmlmin#66

The latest commit in https://github.com/mankyd/htmlmin is from 2017.
Maybe it's time to move to a different html minifier?
Also you could avoid importing a HTML minifier if minify_html option is false (same for the others)

@wilhelmer
Copy link
Collaborator

Yes you're right, the state of htmlmin is not looking very promising. However, it's hard to find a good alternative.

There's minify-html, but it's based on Rust, so you'd have to install Rust and Cargo and whatnot to use mkdocs-minify-plugin. At least that seems to be the case on my M1 Mac.

Then there's django-htmlmin, which seems to be a good choice, despite the name. I managed to get it running with mkdocs-minify-plugin in 1 minute and it seems to work fine. However, their last release is also from >3 years ago.

Also, both minifiers aren't as configurable as htmlmin, so we'd lose some option that people rely on.

Thoughts? Anything else?

@ldeluigi
Copy link
Author

I don't know any alternative unfortunately. You can always maintain your own fork or hard fork of your favorite minifier if you will.

@thatlittleboy
Copy link
Contributor

There's minify-html, but it's based on Rust, so you'd have to install Rust and Cargo and whatnot to use mkdocs-minify-plugin. At least that seems to be the case on my M1 Mac.

@wilhelmer Using the python binding doesn't require the installation of Rust / Cargo, you can just pip install it (from their README):

image


I tried this and it minifies HTML seemingly fine -- I cant' attest to the feature parity w/ htmlmin, though.

Or is it an M1 issue? (I've never had an M1, so I'm not sure). If so, I think we just need to raise a request upstream to provide an M1-compliant python binding, right?

@wilhelmer
Copy link
Collaborator

Hmm. When I run pip install minify-html, it fails and explicitly states that I must install Rust and Cargo:

Collecting minify-html
  Downloading minify_html-0.10.7.tar.gz (136 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 136.0/136.0 kB 2.6 MB/s eta 0:00:00
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error
  
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [8 lines of output]
      Error in sitecustomize; set PYTHONVERBOSE for traceback:
      AssertionError:
      
      Cargo, the Rust package manager, is not installed or is not on PATH.
      This package requires Rust and Cargo to compile extensions. Install it through
      the system's package manager or via https://rustup.rs/
      
      Checking for Rust toolchain....
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

@thatlittleboy
Copy link
Contributor

thatlittleboy commented Jan 23, 2023

Yea, seems like you're downloading the sdist (tar.gz) rather than the prebuilt wheels.

When I pip install in a linux docker image

(base) root@a38cc2049797:/workspaces/pyjanitor# pip install minify-html
Collecting minify-html
  Downloading minify_html-0.10.7-cp39-cp39-manylinux_2_27_x86_64.whl (719 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 719.8/719.8 kB 1.9 MB/s eta 0:00:00
Installing collected packages: minify-html
Successfully installed minify-html-0.10.7

On my 2017 macbook pro (in a python virtualenv)

(scratch.DQ7WPno1) T/scratch.DQ7WPno1 » pip install --no-cache-dir minify-html
Collecting minify-html
  Downloading minify_html-0.10.7-cp39-cp39-macosx_10_7_x86_64.whl (632 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 632.1/632.1 kB 20.7 MB/s eta 0:00:00
Installing collected packages: minify-html
Successfully installed minify-html-0.10.7

What's your ABI/ hardware / python version? Then we can check against the triplet on their PyPI page? If your triplet combination is common enough, I'm sure we can raise an issue upstream to prebuild a wheel for that.

https://pypi.org/project/minify-html/#files


EDIT: okay nvm, the author dropped support for M1 Silicon Macs 5 months ago due to lack of Github runners

@wilhelmer
Copy link
Collaborator

😆 He dropped it on Aug 9, and exactly on the same day, GitHub Action added support for M1 runners.

Maybe you can convince him to add it back?

@thatlittleboy
Copy link
Contributor

@wilhelmer Hah, yea.. but seems like GH only supports self-hosted runners for M1 though, so if the author has no M1 hardware on hand, they'll have to shell money out to use a cloud runner? --- I can try to ping him in the original issue, but I'm not hopeful :p

On a separate note, would you be open to still consider the migration to minify-html given this current limitation? (Especially if you consider this plugin to be more or less stable);
I'm thinking we could do a major release with updated dependencies, and M1 users who have trouble / can't bother with installing cargo can pin dependencies to the older version of mkdocs-minify-plugin?

@wilhelmer
Copy link
Collaborator

Not sure. Is it really worth the hassle just to avoid a deprecation message? It's an INFO message, not a warning, despite the wording, so it won't even affect MkDocs' strict mode (mkdocs build -s).

@thatlittleboy
Copy link
Contributor

thatlittleboy commented Jan 23, 2023

Is it really worth the hassle just to avoid a deprecation message?

Perhaps not at this point in time. It'll only be necessary once py3.13 rolls around , since cgi that htmlmin relies on will no longer be available (?).


Though, on second look, htmlmin doesn't seem to be that reliant on cgi. If it comes down to it, someone could maintain a simple fork of htmlmin that removes the global cgi import and continue to use that for this mkdocs plugin.

@SilentGlasses
Copy link

SilentGlasses commented Mar 7, 2023

Same issue here, I am not a python dev or I would take this on to not wait till hell fire hits home. Not sure why devs tend to kick cans down the road till the last minute. 👅 JK
Any help with figuring this out would be greatly appreciated.

Question, is this s problem with the plugin or mkdocs itself? Seems like the latter.

@kamilkrzyskow
Copy link
Contributor

@SilentGlasses That's more of a "somebody will do it instead of me in the 2 years before it reaches the hell fire" ideology 😅Don't fix what's not broken yet kinda thing😏, and like wilhelmer said previously, it's a non crashing INFO warning, so I'm surprised people are so adamant on having to fix it. I doubt you'll have to wait for long since I've seen @wilhelmer started doing some adjustments in his fork https://github.com/wilhelmer/htmlmin/commits/master .

@wilhelmer
Copy link
Collaborator

Yeah I fixed it in my own fork and it works, but I'm still reluctant to change the dependency. Maybe in a few weeks or months 😄 But don't worry, there will be no hell fire, we can switch this very easily.

Question, is this s problem with the plugin or mkdocs itself? Seems like the latter.

No, it's a problem with the plugin (or rather, one of its dependencies). However, in earlier versions of MkDocs, you wouldn't have noticed this. Now, MkDocs displays these DeprecationWarnings at build time (see mkdocs/mkdocs#2907).

@squidfunk
Copy link

Also really looking for a fix! I suspect that this will upset many users. Deprecation warnings are kid of meh due to their unpredictable nature when they will become errors and crash the build, especially with minimum-only version specs.

@wilhelmer
Copy link
Collaborator

Okay okay, let's go with my fork then. Should be the safest option without introducing any breaking changes or losing any HTML minification options.

@wilhelmer
Copy link
Collaborator

@byrnereese Please release v0.6.3 on PyPI, thanks 🙏

@byrnereese
Copy link
Owner

@wilhelmer 0.6.3 released!

@wilhelmer
Copy link
Collaborator

Argh, I forgot to update the htmlmin dependency in setup.py. Only looked in requirements.txt.

@byrnereese We need another release (0.6.4), sorry 🙏

@byrnereese
Copy link
Owner

@wilhelmer I having an issue uploading the new build. I get the following:

Uploading mkdocs_minify_plugin-0.6.4-py3-none-any.whl
100% ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 10.1/10.1 kB • 00:00 • 4.9 MB/s
WARNING Error during upload. Retry with the --verbose option for more details.
ERROR HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/
Invalid value for requires_dist. Error: Can't have direct dependency: 'htmlmin @ git+https://github.com/wilhelmer/htmlmin.git@master'

@wilhelmer
Copy link
Collaborator

Jeez, seems like PyPI doesn't allow git dependencies. I created a new package called htmlmin2 containing the forked version.

Please pull the latest changes and try uploading 0.6.4 again.

@kamilkrzyskow
Copy link
Contributor

kamilkrzyskow commented Mar 14, 2023

😮Interesting, I also thought that the git fork dependency would work. Reading this thread pypa/pip#6301 I got the weird feeling that it's possible to influence the installed packages in the setup.py, but that doesn't seem to be the case, or I'm not skilled enough😞So I made a hack in the plugin file,

from os.path import dirname, join as join_paths
import jsmin
# ...

escape_path = join_paths(dirname(dirname(jsmin.__file__)), "htmlmin", "escape.py")
main_path = join_paths(dirname(dirname(jsmin.__file__)), "htmlmin", "main.py")

deprecated_escape = """try:
  from html import escape
except ImportError:
  from cgi import escape"""
solution_escape = "from html import escape"
deprecated_main = "import cgi"
solution_main = ""

with open(escape_path, encoding="utf-8-sig") as file:
    escape_source = file.read()

with open(main_path, encoding="utf-8-sig") as file:
    main_source = file.read()

# Write new content to the source files

with open(escape_path, "w", encoding="utf8") as file:
    file.write(escape_source.replace(deprecated_escape, solution_escape))

with open(main_path, "w", encoding="utf8") as file:
    file.write(main_source.replace(deprecated_main, solution_main))

# Import the module after it's been modified to avoid any DeprecationWarning

import htmlmin

# ...

This would add some read/write cycles during the import of the plugin, but would ultimately fix the installed files, before the htmlmin import. However, this is just a hack limited to this plugin only.
I do like the approach with the public separate "patch" fork more 👍, because people can install it to other projects as well. I would personally call it htmlmin-without-cgi / htmlmin-undeprecated tough ✌️, since it's just a fix and not a completely new version 2.0.

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

7 participants