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

Keep relative links with rel="relative" attribute #1728

Closed

Conversation

W4RH4WK
Copy link

@W4RH4WK W4RH4WK commented Sep 24, 2022

This allows me to link to other PDFs with a relative link without fiddling with the base URL. See #532 for details.

@W4RH4WK
Copy link
Author

W4RH4WK commented Sep 24, 2022

Please note that I was unable to run the test suite as described in the documentation.

$ venv/bin/pip --version
pip 20.0.2 from /mnt/c/Users/w4rh4wk/git/WeasyPrint/venv/lib/python3.8/site-packages/pip (python 3.8)

$ venv/bin/pip install -e .[doc,test]
ERROR: File "setup.py" not found. Directory cannot be installed in editable mode: /mnt/c/Users/w4rh4wk/git/WeasyPrint
(A "pyproject.toml" file was found, but editable mode currently requires a setup.py based build.)

@grewn0uille
Copy link
Member

Hello,

To install things in an editable way without setup.py, you need pip >= 21.3.

@W4RH4WK W4RH4WK force-pushed the prefer-relative-links-when-allowed branch from bac8400 to 1b470dc Compare September 25, 2022 14:08
@W4RH4WK W4RH4WK changed the title Prefer relative links when allowed Keep relative links with rel="relative" attribute Sep 25, 2022
@W4RH4WK
Copy link
Author

W4RH4WK commented Sep 25, 2022

Digging it seems like this should be parameterized in a way. We probably don't want to change the default behavior.

I looked into adding a new commandline flag for this, but forwarding that flag to the place where it's needed seems to be a nightmare.

While trying that, I discovered the special handling for attachments using rel="attachment" and thought, that this might be an easier and more flexible solution.

So I changed the PR to not resolve relative links when they have a rel="relative" attribute.


I've now installed Python 3.10 and are using pip 22.2.1, but the pip install call still fails.

> venv\Scripts\pip.exe install -e .[doc,test]
Obtaining file:///C:/Users/w4rh4wk/git/WeasyPrint
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Installing backend dependencies ... done
  Preparing editable metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing editable metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [34 lines of output]
      Traceback (most recent call last):
        File "C:\Users\w4rh4wk\git\WeasyPrint\venv\lib\site-packages\pip\_vendor\pep517\in_process\_in_process.py", line 363, in <module>
          main()
        File "C:\Users\w4rh4wk\git\WeasyPrint\venv\lib\site-packages\pip\_vendor\pep517\in_process\_in_process.py", line 345, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "C:\Users\w4rh4wk\git\WeasyPrint\venv\lib\site-packages\pip\_vendor\pep517\in_process\_in_process.py", line 191, in prepare_metadata_for_build_editable
          return hook(metadata_directory, config_settings)
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\overlay\Lib\site-packages\flit_core\buildapi.py", line 49, in prepare_metadata_for_build_wheel
          metadata = make_metadata(module, ini_info)
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\overlay\Lib\site-packages\flit_core\common.py", line 408, in make_metadata
          md_dict.update(get_info_from_module(module, ini_info.dynamic_metadata))
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\overlay\Lib\site-packages\flit_core\common.py", line 205, in get_info_from_module
          docstring, version = get_docstring_and_version_via_import(target)
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\overlay\Lib\site-packages\flit_core\common.py", line 178, in get_docstring_and_version_via_import
          spec.loader.exec_module(m)
        File "<frozen importlib._bootstrap_external>", line 883, in exec_module
        File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
        File "C:\Users\w4rh4wk\git\WeasyPrint\weasyprint\__init__.py", line 345, in <module>
          from .css import preprocess_stylesheet  # noqa isort:skip
        File "C:\Users\w4rh4wk\git\WeasyPrint\weasyprint\css\__init__.py", line 25, in <module>
          from . import computed_values, counters, media_queries
        File "C:\Users\w4rh4wk\git\WeasyPrint\weasyprint\css\computed_values.py", line 9, in <module>
          from ..text.ffi import ffi, pango, units_to_double
        File "C:\Users\w4rh4wk\git\WeasyPrint\weasyprint\text\ffi.py", line 398, in <module>
          gobject = _dlopen(
        File "C:\Users\w4rh4wk\git\WeasyPrint\weasyprint\text\ffi.py", line 385, in _dlopen
          return ffi.dlopen(names[0])  # pragma: no cover
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\normal\Lib\site-packages\cffi\api.py", line 150, in dlopen
          lib, function_cache = _make_ffi_library(self, name, flags)
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\normal\Lib\site-packages\cffi\api.py", line 832, in _make_ffi_library
          backendlib = _load_backend_lib(backend, libname, flags)
        File "C:\Users\w4rh4wk\AppData\Local\Temp\pip-build-env-urpb_yxf\normal\Lib\site-packages\cffi\api.py", line 827, in _load_backend_lib
          raise OSError(msg)
      OSError: cannot load library 'gobject-2.0-0': error 0x7e.  Additionally, ctypes.util.find_library() did not manage to locate a library called 'gobject-2.0-0'
      [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.

[notice] A new release of pip available: 22.2.1 -> 22.2.2
[notice] To update, run: C:\Users\w4rh4wk\git\WeasyPrint\venv\Scripts\python.exe -m pip install --upgrade pip

It appears that there are other system dependencies that I do not have installed.

@liZe
Copy link
Member

liZe commented Sep 25, 2022

Hi!

Thanks a lot for the pull request.

Digging it seems like this should be parameterized in a way. We probably don't want to change the default behavior.

I looked into adding a new commandline flag for this, but forwarding that flag to the place where it's needed seems to be a nightmare.

While trying that, I discovered the special handling for attachments using rel="attachment" and thought, that this might be an easier and more flexible solution.

So I changed the PR to not resolve relative links when they have a rel="relative" attribute.

This problem is complex, and it could be a good idea to discuss about the problem and find a "good" solution before trying to solve it with code 😁️.

A very "good" way to solve this would be:

  • to give you a way to do what you want to do,
  • to keep the current behavior for documented and tested cases,
  • to respect the standards.

The current PR breaks tests (but I think that it’s not intended), and the rel="relative" attribute is not in the standards. I’m afraid we have to find a better way 😀️.

We added the support rel="attachment" a loooooong time ago, when WeasyPrint was a tiny little project, after a long discussion. attachment was already used and proposed for standardization. rel is supposed to define the type of link, which is OK for attachment but a little bit strange for relative in my opinion.

Another solution would be to support an HTML base attribute, just as xml:base for XML. Unfortunately, the attribute is not allowed in HTML, it’s been proposed but rejected mainly for performance issues.

It could be useful to see what other HTML-to-PDF tools do. Prince seems to have a non-standard -prince-pdf-link-type property that forces links to lead to local files and ignore the base URL. It’s also interesting to note that it doesn’t automatically find the base URL when a relative path is given: prince link.html -o link.pdf creates relative links, while prince file:///path/to/link.html -o link.pdf creates absolute links. There’s no right or wrong way to handle this, and even if the "feature" is useful in this case it may be seen as a bug in other cases. That’s the same problem with browsers resolving (or not) relative URLs when downloading web pages. Some think it’s a bug, some think it’s a feature, it depends on what the user is trying to do.

So… Before solving this problem, it could be useful to take some time to find what’s done by the other tools, what’s been proposed/accepted/rejected by the specifications, and what could give users a large panel of possibilities without impacting the current behavior. It may take some time, it can definitely be frustrating 😀️, but it’s a good solution to get nice, solid, useful and maintainable features.

Technically, the CSS property seems to be a really good idea, even better that the good HTML attribute idea. But it’s not included (or even mentioned) in any standard as far as I can tell, and that’s generally a sufficient reason to reject the solution. We’ll probably have to find something better than this…

It appears that there are other system dependencies that I do not have installed.

Following the installation steps should be enough. GObject is provided by the GTK installer.

@W4RH4WK
Copy link
Author

W4RH4WK commented Sep 25, 2022

Yeah, I see where you are coming from. Thanks for the detailed response, maybe I'll find the time to look into this.

@liZe
Copy link
Member

liZe commented Aug 20, 2023

Don’t hesitate to open a new pull request if you find time to work on this! 💜

@liZe liZe closed this Aug 20, 2023
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

Successfully merging this pull request may close these issues.

4 participants