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

Special !! values are reset to null in mkdocs.yml #7461

Closed
pawamoy opened this issue Sep 5, 2020 · 7 comments · Fixed by #7507
Closed

Special !! values are reset to null in mkdocs.yml #7461

pawamoy opened this issue Sep 5, 2020 · 7 comments · Fixed by #7507
Labels
Bug A bug Needed: design decision A core team decision is required

Comments

@pawamoy
Copy link
Contributor

pawamoy commented Sep 5, 2020

Details

As reported here (the issue is now stale, not sure if you'd prefer the discussion to continue over there), values like !!python/name:module.name in mkdocs.yml are dropped during the build process. They end up being null in the final mkdocs.yml, which in turn breaks build in various places (mainly plugins).

Expected Result

These values should not be reset to null as it breaks the build. More details in the linked issue.

Actual Result

Special values starting with !!python/name: in mkdocs are reset to null, and it breaks the build.

@stsewd
Copy link
Member

stsewd commented Sep 8, 2020

The solution in #6889 (comment) should do the trick. We parse those inside a docker environment, so it should be safe.

@stsewd stsewd added Accepted Accepted issue on our roadmap Bug A bug labels Sep 8, 2020
@stsewd
Copy link
Member

stsewd commented Sep 17, 2020

So, we execute that in the builders, but the parsing code is still executed inside the application layer, we are safe to execute commands that we send to docker, not the ones we execute inside the application layer.

So, we can't use the normal parse function here. Some options:

  • Make this a script that would be executed inside the docker container.
  • Find another way to parse the yaml file without executing random code and without modifying the original code (maybe another library?).
  • Plugins shouldn't rely on that and find another way for users to set such options.

@stsewd stsewd added Needed: design decision A core team decision is required and removed Accepted Accepted issue on our roadmap labels Sep 17, 2020
@pawamoy
Copy link
Contributor Author

pawamoy commented Sep 23, 2020

Here's a way to "fake" load !!python/name: tags without losing their value when redumping them (second option above):

from pathlib import Path
import yaml


class ProxyPythonName(yaml.YAMLObject):
    def __init__(self, value):
        self.value = value


class CustomLoader(yaml.SafeLoader):
    def construct_python_name(self, suffix, node):
        return ProxyPythonName(suffix)


class CustomDumper(yaml.SafeDumper):
    def represent_name(self, data):
        return self.represent_scalar("tag:yaml.org,2002:python/name:" + data.value, "")


CustomLoader.add_multi_constructor("tag:yaml.org,2002:python/name:", CustomLoader.construct_python_name)
CustomDumper.add_representer(ProxyPythonName, CustomDumper.represent_name)

string = Path("mkdocs.yml").read_text()
loaded = yaml.load(string, Loader=CustomLoader)
dumped = yaml.dump(loaded, Dumper=CustomDumper)
print(dumped)

Input:

markdown_extensions:
  - pymdownx.superfences:
      custom_fences:
        - class: mermaid
          format: !!python/name:pymdownx.superfences.fence_div_format
          name: mermaid

Output:

markdown_extensions:
- pymdownx.superfences:
    custom_fences:
    - class: mermaid
      format: !!python/name:pymdownx.superfences.fence_div_format ''
      name: mermaid

No Python code was imported / executed 🙂

@FeeeeK
Copy link

FeeeeK commented Apr 17, 2023

Two years later, the problem reappeared again, but this time with !!python/object/apply, needed for pymdown-extensions:
https://facelessuser.github.io/pymdown-extensions/faq/#function-references-in-yaml

@pawamoy
Copy link
Contributor Author

pawamoy commented Apr 17, 2023

Shouldn't be too hard to open a new issue asking if maintainers would be OK to support it, and to copy-paste the code of the previous PR adding support for name and change it a bit to support apply.

@humitos
Copy link
Member

humitos commented Apr 17, 2023

@FeeeeK it seems we implemented this only for !!python/name, see #7507 (comment). I think we could expand that list now that there is a needing for this.

As a workaround for now, you could use the feature to Override the build process by using build.commands config key. This way, Read the Docs won't do anything with your mkdocs.yml file and you MkDocs plugin will work.

Here is an example of how to create the .readthedocs.yaml file using this approach:

version: 2

build:
  os: ubuntu-22.04
  tools:
    python: "3"
  commands:
    pip install -r requirements.txt
    mkdocs build --site-dir $READTHEDOCS_OUTPUT/html

@FeeeeK
Copy link

FeeeeK commented Apr 17, 2023

Thanks, @humitos, I already did it, but it would be nice if there were any warnings about this, now there is only traceback from toc without any additional info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug Needed: design decision A core team decision is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants