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

Update page.file source path with original source, fixes #12 #13

Merged
merged 8 commits into from
Jun 3, 2020

Conversation

timvink
Copy link
Contributor

@timvink timvink commented Apr 18, 2020

This approach uses the on_pre_page event to update the page absolute source path attribute. It should fix issues when combining mkdocs-monorepo-plugin with plugins that depend on that attribute, like mkdocs-git-authors-plugin and mkdocs-git-revision-date-localized-plugin.

@bih
Copy link
Collaborator

bih commented Apr 19, 2020

@timvink Thanks for the PR! Looks good to me.

Before I merge, there's two things:

  • Version change. Bump up the version in /setup.py (I think right now it should be 0.4.5 given it isn't a breaking change). A simple change note in docs/CHANGELOG.md would be good too.
  • Test coverage. Add test cases against the two plugins (i.e. mkdocs-git-authors-plugin and mkdocs-git-revision-date-localized-plugin). It would be good to ensure that over time we can retain compatibility with these two plugins. I would suggest two - one for each plugin. You should be able to easily add them in our test.bats file and then by making two folders (just copy the ok folder as a base) in our fixtures folder. You might need to add those dependencies to our root requirements.txt file too (linked here).

Let me know if you need more guidance, hoping we can ship this change soon! :)

@bih bih added the bug Something isn't working label Apr 19, 2020
@timvink
Copy link
Contributor Author

timvink commented Apr 20, 2020

Thanks for the guidance!

I added the unit tests. I had to add apt-get install git to the unit test setup in order for the tests to run.

Note that I fixed the versions of the plugins in requirements.txt.. I don't want to break any CI in case of future updates. I would recommend to consider setting up https://dependabot.com/ for this project, you will get a MR from a bot as soon as there are any updates to dependencies.

Let me know if there is anything else I can improve :)

@prcr
Copy link

prcr commented Apr 28, 2020

Hi @bih, is there any chance we could move this solution forward? Can I help in any way?

@bih bih merged commit 6b53d8f into backstage:master Jun 3, 2020
@bih
Copy link
Collaborator

bih commented Jun 3, 2020

@pauloribeiro-codacy @timvink Finally got around to merging this. Sorry for the waiting on your side folks! :-)

@rossmechanic
Copy link
Contributor

Just tried updating to this. It was working fine before this change, and now I'm getting a KeyError on one of my docs:

KeyError: '/var/folders/z6/gndttzfn5z34y_wkpmm9fyyw0000gp/T/docs_373fzmc5/best_practices/README.md'

@bih any idea?

@prcr
Copy link

prcr commented Jun 4, 2020

I also ran into the same issue. Here's the traceback:

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.3/x64/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/mkdocs/__main__.py", line 159, in build_command
    build.build(config.load_config(**kwargs), dirty=not clean)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/mkdocs/commands/build.py", line 274, in build
    _populate_page(file.page, config, files, dirty)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/mkdocs/commands/build.py", line 163, in _populate_page
    page = config['plugins'].run_event(
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/mkdocs/plugins.py", line 94, in run_event
    result = method(item, **kwargs)
  File "/opt/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/mkdocs_monorepo_plugin/plugin.py", line 62, in on_pre_page
    page.file.abs_src_path = self.files_source_dir[page.file.abs_src_path]
KeyError: '/tmp/docs_senr2psd/Chart/configuration/monitoring.md'

@timvink
Copy link
Contributor Author

timvink commented Jun 4, 2020

Hmm, that's not good.

I'm having trouble reproducing this locally. Which plugins do you have activated? Doing mkdocs serve or mkdocs build? And if it's a small site, a tree of the structure might also help to pinpoint the issue.

Somehow these files were not registered when the mono repo merged the docs:

https://github.com/timvink/mkdocs-monorepo-plugin/blob/ba2e410d811bf5566a677e41373a6d5c479bd630/mkdocs_monorepo_plugin/merger.py#L67

I need to figure out why..

update: OK i figured out the problem: it has to due with folders inside a docs folder. None of the unit tests have this, which is why I didn't catch it earlier. Will now find cause + fix it..

@prcr
Copy link

prcr commented Jun 4, 2020

Yes, I was also noticing this. In the traceback the temporary folder mentions:

/tmp/docs_xxxxxxxx/Chart/configuration/monitoring.md

But the Markdown file is actually in a /docs folder inside the "Chart" submodule repository:

/docs/configuration/monitoring.md

@timvink timvink mentioned this pull request Jun 4, 2020
@timvink
Copy link
Contributor Author

timvink commented Jun 4, 2020

I found the root cause & created a hotfix to solve it: #19. Sorry for missing this :(

@bih hope you can review + merge quickly, as we now have a bug in production..

@prcr
Copy link

prcr commented Jun 5, 2020

I believe we still have an incompatibility with the git-revision-date-localized plugin. When I have both plugins activated, Git does not detect the temporary folder as a local Git repository:

Traceback (most recent call last):
  File "/home/prcr/.local/bin/mkdocs", line 11, in <module>
    sys.exit(cli())
  File "/home/prcr/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/prcr/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/prcr/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/prcr/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/prcr/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/prcr/.local/lib/python3.6/site-packages/mkdocs/__main__.py", line 152, in build_command
    build.build(config.load_config(**kwargs), dirty=not clean)
  File "/home/prcr/.local/lib/python3.6/site-packages/mkdocs/commands/build.py", line 236, in build
    config = config['plugins'].run_event('config', config)
  File "/home/prcr/.local/lib/python3.6/site-packages/mkdocs/plugins.py", line 94, in run_event
    result = method(item, **kwargs)
  File "/home/prcr/.local/lib/python3.6/site-packages/mkdocs_git_revision_date_localized_plugin/plugin.py", line 36, in on_config
    self.util = Util(path=config["docs_dir"])
  File "/home/prcr/.local/lib/python3.6/site-packages/mkdocs_git_revision_date_localized_plugin/util.py", line 14, in __init__
    git_repo = Repo(path, search_parent_directories=True)
  File "/home/prcr/.local/lib/python3.6/site-packages/git/repo/base.py", line 181, in __init__
    raise InvalidGitRepositoryError(epath)
git.exc.InvalidGitRepositoryError: /tmp/docs_o4kb_ykw

This is just a wild guess, but could it be that the .git folder is not being copied to the temporary folder?

@timvink
Copy link
Contributor Author

timvink commented Jun 5, 2020

OK that's unfortunate. Thanks for the persistent reports, really helpful !

I did some more research and here's what's going on:

1. bats testing not working

All units tests pass, but the follow fails with the InvalidGitRepositoryError error:

mkdocs build -f __tests__/integration/fixtures/ok-mkdocs-git-revision-date-localized-plugin/mkdocs.yml

--> @bih Perhaps this is something you can fix on your end, as it seems like something is wrong with bats? Not urgent though.

2. Finding the .git folder

mkdocs-git-authors-plugin works fine with monorepo now, but mkdocs-git-revision-date-localized-plugin gives InvalidGitRepositoryError . Here's why:

  • revision-date looks in the config["docs_dir"] folder for .git, and then recursively looks up. It does this on the on_config() plugin event. This is because some users have their .git folder inside the docs/ folder (f.e. when using git submodules), context in this PR. monorepo changes the value of the docs_dir to the temporary folder, so it's looking in the wrong place.
  • git-authors stores the current working directory on the plugin's __init__, and recursively looks upward for a .git folder. In the future, it should also use the docs_dir for the same reason as the revision-date plugin.

A potential solution should be able to access config["docs_dir"] before monorepo changes it. mkdocs runs plugin events in the order they are defined. So the solution is actually quite simple; change in your mkdocs.yml the ordering from:

plugins:
  - monorepo
  - git-revision-date-localized

to:

plugins:
  - git-revision-date-localized
  - monorepo

--> On my side I will work on a new release of mkdocs-git-revision-date-localized-plugin that raises an instructional error if the plugin ordering is wrong. I opened timvink/mkdocs-git-revision-date-localized-plugin#34 for that.

@prcr
Copy link

prcr commented Jun 8, 2020

Thanks @timvink, the workaround of reordering the plugins in the mkdocs.yml works for me. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants