Use git config values as default author and email

[NMertsch]

Summary

This PR improves the default values for "Author" and "Author's Email" in briefcase new and briefcase convert by reading the user's git configuration.

Fixes #2269

Why?

Currently, the default values are:

• Author: Jane Developer
• Author's Email: firstname@domain (e.g. [email protected] from Author "Niklas Mertsch" and domain "org.example")

This PR checks for the git config values user.name and user.email. If they are set, they are used instead of the current default values.

Details

New method BaseCommand.get_git_config_value(section: str, option: str) -> str | None

• Read and merge from the following files, if present:
  • system config (/etc/git/config)
  • global config (~/.gitconfig)
  • user config (~/.config/git/config)
  • repo config (./.git/config)

Updated default values in NewCommand and ConvertCommand

• If self.get_git_config_value("user", "name"/"email") is not None:
  • Use it as default value.
  • Add sentence "Based on your git configuration, we believe it could be '{value}'." to the prompt intro (wording copied from ConvertCommand email handling).
• Otherwise, use the current default value (e.g. "Jane Developer").

Comments for reviewers

• I chose to only use the git username if no "PEP 621 'authors'" were found.
• I don't know if BaseCommand really is the right place for get_git_config_value(). I chose it because it is the base class of NewCommand and ConvertCommand, and because it provides self.tools.git so we don't have to guard the git import in multiple places.
• I chose not to test that get_git_config_value() does what it is doing. The implementation is trivial, and I can't find a way to test it without actually just testing mocks.
• I chose to test the new author and email functionality for NewCommand together, not in one test for author and one for email. That's because both are set in the same method (NewCommand.build_app_context()), so splitting the tests seemed to add too much code duplication. Splitting build_app_context() into smaller methods might be the right approach here, but I didn't want to take that decision for you.

Open questions for reviewers

• The UX improvement is nice to have, but I'm not satisfied with the complexity my PR adds to the repo. What do you think?
• I don't understand if/where the default values from briefcase-template are used. Can I ignore that repo for this change?
• To keep command.tools.git out of the general test setup, I check for its presence in get_git_config_value(). This check should always be true, so I added self.console.warning(...) in that case. Is that the right way to handle this?

PR Checklist:

• Implement feature
  • Implement git config as optional source for default values
  • Tell users where the default values are coming from
• All new features have been tested
  • get_git_config_value() reads correct config files
  • NewCommand uses git values if present
  • ConvertCommand uses git values if present
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[rmartin16]

I think this is a cool and clever idea....but I'll just throw this out there to consider, @freakboy3742.

There may be somewhat of a "creepy factor" here. When one uses a tool that starts using their personal information without permission or explaining itself, it might feel creepy/invasive. Or it might conjure questions of "what other information is being gathered?" Or "is this information being exfiltrated anywhere?"

If there's any agreement this is more of a valid concern rather than paranoia, it may be worthwhile to tell the user these details were simply retrieved from Git's settings when they are shown.

Coming from another direction, if the Git data is inaccurate, then the user may just be confused....for instance, what if this is a shared computer and someone else's details get pulled in.

[freakboy3742]

@NMertsch Thanks for the PR; it's currently getting stuck on the towncrier requirement in CI; see the contribution guide for details on how to resolve this.

There may be somewhat of a "creepy factor" here. When one uses a tool that starts using their personal information without permission or explaining itself, it might feel creepy/invasive. Or it might conjure questions of "what other information is being gathered?" Or "is this information being exfiltrated anywhere?"

This feels a little paranoid, but if it's triggered your paranoia filter, then you won't be the only one. If we've used git details, it might be worth adding a "Based on your git configuration, we believe your name is..." comment to the help text where we're using those details. That also helps mitigate the "someone else's computer" problem.

[NMertsch]

Thank you for the quick feedback!

There may be somewhat of a "creepy factor" here.

Good point, I didn't consider this and totally agree about the creepiness. I'll add a note about where the information is coming from (briefcase convert already has this in part).

Thanks for the PR; it's currently getting stuck on the towncrier requirement in CI

I know, and I'm also missing most tests, and am uncertain about some design decisions. That's why I marked the PR as Draft and added some questions 🙂 I'll mark it as Ready once I'm done.

[freakboy3742]

This looks really good - and it works great in my manual testing.

A couple of cosmetic tweaks to language in the UI and changenote; and some small suggestions for test improvements.

[freakboy3742]

This all looks great - with one last change to remove the "if no git" section in favor of. test mock for get_git_config_value(), I think this will be ready to land!

[freakboy3742]

This looks great - thanks for those updates.

The CI failures look to be the semi-regular "OpenSuSE messes up their package repo" bug that we see occasionally; that usually self-resolves with 24 hours. I'll keep an eye on it and push the "retry" button until it works.