timvink / mkdocs-git-revision-date-localized-plugin

MkDocs plugin to add a last updated date to your site pages
https://timvink.github.io/mkdocs-git-revision-date-localized-plugin/index.html
MIT License
193 stars 39 forks source link

``InvalidGitRepositoryError`` during build when including auto-generated files #101

Closed Teagum closed 1 year ago

Teagum commented 1 year ago

I tried to use mkdocs-git-revision-date-localized-plugin together with [mkdocs-gen-files] to auto-generate references from Python doc strings. During the build, I encountered the following error message:

[git-revision-date-localized-plugin] Unable to find a git directory and/or git is not installed. To ignore this error, set option 'fallback_to_build_date: true',

followed by a git.exc.InvalidGitRepositoryError.

The fundamental problem is that mkdocs-gen-files stores its output in temporary files outside the project's repository. Hence, git-revision-date-localized cannot find a repository and produces the error.

On the other hand, it is possible to silence the error by setting fallback_to_build_date: true in mkdocs.yaml, such that the auto-generated pages are marked with the build date. This, however, turns the errors for each file into warnings, and thus it is impossible to use mkdocs in --strict mode.

Also, the exclude option does not work in this case since the names of the temporary files are unknown before build time.

I currently don't have a proper solution since it is obviously not straightforward to map auto-generated temporary files to the corresponding code files.

timvink commented 1 year ago

Exclude supports globbing, do you know the directory ?

So you want to use --strict.. would a new option not to raise any warnings help?

Teagum commented 1 year ago

Exclude supports globbing, do you know the directory ?

No, I don't know the directory. It has a random identifier that changes on every run of mkdocs build

would a new option not to raise any warnings help?

I believe it would be best not to raise a warning in this case. The behavior has to be requested explicitly by setting fallback_to_build_date: true, so users should know what to expect. I would use warnings only for situations that may cause unintended behavior.

What do you think about it? If you agree to remove the warning, I'll be happy to prepare a pull request.

timvink commented 1 year ago

to auto-generate references from Python doc strings

Not sure if you're aware, but checkout mkdocstrings if you haven't already.

Another thought. What does your mkdocs.yml plugin section look like? I suspect you have gen-files before this plugin. Can you try switching the order? If that solves the problem, I can use the new (as of mkdocs 1.4) event priorities to ensure correct execution order.

But that's unlikely to work, considering the event tree: gen-files uses the on_files event, and this plugin mainly uses the on_page_markdown event.

I still think the proper way is to exclude certain files and/or directories from this plugin. This is the logic currently used:

https://github.com/timvink/mkdocs-git-revision-date-localized-plugin/blob/ad11dd08a80422656e64fe56a6044fcec3b2742e/mkdocs_git_revision_date_localized_plugin/plugin.py#L167-L169

It might be that these generated files do not have a page.file.src_path attribute. But that seems unlikely because the files are generated properly using the mkdocs File class, so that attribute should be set: https://github.com/oprypin/mkdocs-gen-files/blob/b1d62c1e3193ddddb2f33599c4538792e7e838d5/mkdocs_gen_files/editor.py#L42-L47

A more obvious route might to add a PR to mkdocs-gen-files-plugin that adds an attribute to generated pages, something inserting after this line the following:

new_f.generated_by_gen_files = True

Then we can build support for gen-files into this plugin by testing during exclusion if hasattr(page.file, "generated_by_gen_files"). That way, 1) we avoid adding extra options to this plugin, 2) new users will not have to worry about the setup but it works first time.

Thoughts?

Teagum commented 1 year ago

Not sure if you're aware, but checkout mkdocstrings if you haven't already.

I use gen-files and mkdocstrings precisely as described in this recipe.

I suspect you have gen-files before this plugin.

I tried all possible positions before posting this issue. The error is always the same regardless of where I place git-revision-date-localized in the plugins section.

I'll try what you've proposed above later today or tomorrow.

timvink commented 1 year ago

I've opened two PRs that will address this issue. Waiting for the PR to gen-files to be merged & released before I can add support to this plugin

squidfunk commented 1 year ago

I would like to reopen this topic. We should either adopt the generated_by field as a new field in File over at MkDocs, or this plugin should check if a file is actually located inside the project directory before trying to get the git history. I've just refactored the blog plugin and kind of regard this as a hack, as we're adding a field that is not part of the typings.

Checking if a file is located inside the project:

import os

def is_in_project(self, file: File, config: MkDocsConfig):
    path = os.path.dirname(config.config_file_path)
    return file.abs_src_path.startswith(path)

What do you think? I don't expect MkDocs adopting this in the near future.

timvink commented 1 year ago

It would make sense: if we can't find the file, then don't attempt to use git. But using the config_file_path to determine the directory is also hacky; there are too many edge cases. Top of mind, some things that could happen:

I could spend more time looking for examples and corresponding reasons why, to see if these hold, but I currently don't trust the location of the config enough to infer location of other files.

MkDocs to adopting a generated_by would be a better solution. I've opened https://github.com/mkdocs/mkdocs/pull/3344 to start the discussion there.

squidfunk commented 1 year ago

Thanks for taking the time to investigate and the work on the PR! I understand that explicit is always better than implicit, and adding the respective field would also allow other plugins to adjust more easily.