jimporter / mike

Manage multiple versions of your MkDocs-powered documentation via Git
BSD 3-Clause "New" or "Revised" License
551 stars 47 forks source link

Align git config user.name / user.email with MkDocs #98

Closed ktomk closed 2 years ago

ktomk commented 2 years ago

Requesting to be able to run mike deploy with empty user.name and user.email:

MkDocs does happily deploy with empty /unset git config user.name and user.email.

Currently Mike produces errors when user.name and user.email are unset or empty:

# command: mike deploy 0.1 stable --push --force
error: error getting config 'user.name'
Error: Process completed with exit code 1.

If empty values would not be an error condition, not only would the build with Mike work where MkDocs was in action previously directly (YMMV), it would also allow to keep the same author-information with "mkdocs" build commits when using empty values previously, so less interruption in the meta-data as well.

jimporter commented 2 years ago

Do you mean respecting GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL? That appears to be what ghp_import does (which is what mkdocs gh-deploy uses to generate commits). I don't see anything else that does anything with committer names/emails. See here: https://github.com/c-w/ghp-import/blob/7e554911902433cda8a0cd613ef14fc6b51992f6/ghp_import.py#L130-L131

ktomk commented 2 years ago

Yes, respecting GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL empty or unset as well would be nice. And IIRC ghp_import is also using git-fast-import(1) under the hood.

ktomk commented 2 years ago

It did not error when the config was not set. In my use-case I was only replacing:

mkdocs gh-deploy --force

with

mike deploy 0.1 stable --push --force

not many other effective changes (ref / build error)

jimporter commented 2 years ago

Yes, respecting GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL empty or unset as well would be nice. And IIRC ghp_import is also using git-fast-import(1) under the hood.

Ok, that seems reasonable. We can just do what ghp_import does here, since mike already uses git-fast-import under the hood too (in fact, mike's Git code is a rewrite of ghp_import, so it works roughly the same in terms of what Git commands it runs).

EDIT: I'm not sure this would work if GIT_COMMITER_(NAME|EMAIL) and user.(name|email) are empty, but looking at the code for ghp_import, I think it would complain about that too.

ktomk commented 2 years ago

EDIT: I'm not sure this would work if GIT_COMMITER_(NAME|EMAIL) and user.(name|email) are empty, but looking at the code for ghp_import, I think it would complain about that too.

Well, from what I could observe, mkdocs gh-deploy did not complain, as both the environment variables pair as well as the git config pair were both and all unset. @jimporter

This looks to be the default Github Actions environment / git config.

FYI, I did the following trouble-shooting steps:

  1. changed the github checkout action from v2 to v3: no changes (not working)
  2. first I tried with the environment variables GIT_COMMITER_(NAME|EMAIL) with non-empty fake data: not working
  3. then I tried with git config, set but set to zero-length strings (empty): not working
  4. finally with non-empty git config user.{name,email}: working

The last one, 3., was also confirmed by jimporter read-me in the CI section.

jimporter commented 2 years ago

This should be fixed by 2faa33ca4cf0da62c9c0ac7fbe8b60688733d7f1. mike now accepts unset committer emails, and follows the Git porcelain commands' logic that invalid characters (<, >, \n) in the committer name/email are stripped out.

ktomk commented 2 years ago

Tested 2faa33c, can confirm fixed.

+1 for GIT_COMIMITER_DATE, also tested, see #99 and #100 for an improvement suggestion in terms of supported date formats.