Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Use environment variables when creating signatures#6706

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethomson merged 7 commits intolibgit2:mainfromu-quark:signature-use-env-vars
Jul 10, 2024

Conversation

u-quark
Copy link
Contributor

When creating an action signature (e.g. for a commit author and committer) read the following environment variables that can override the configuration options:

  • GIT_AUTHOR_NAME is the human-readable name in the "author" field.
  • GIT_AUTHOR_EMAIL is the email for the "author" field.
  • GIT_AUTHOR_DATE is the timestamp used for the "author" field.
  • GIT_COMMITTER_NAME sets the human name for the "committer" field.
  • GIT_COMMITTER_EMAIL is the email address for the "committer" field.
  • GIT_COMMITTER_DATE is used for the timestamp in the "committer" field.
  • EMAIL is the fallback email address in case the user.email configuration value isn't set. If this isn't set, Git falls back to the system user and host names.

This is taken from the git documentation chapter "10.8 Environment Variables":

https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables

This PR adds support for reading these environment variables by adding two new functionsgit_signature_default_author andgit_signature_default_committer and deprecates thegit_signature_default function.

Fixes:#3751

Prior work:

@ethomson
Copy link
Member

Thank you for the PR - I agree that we need this functionality. I took a quick first pass on the changes and overall, this is an excellent PR. I will take a closer look 🔜 — thanks again.

@u-quark
Copy link
ContributorAuthor

Hi, thanks for looking into this!

I am also keen on getting this functionality merged! Please let me know if you want a different approach of some specific implementation on this PR, I don't mind changing it.

I am not sure I recreated 100% the CI build environment but I managed to compile with clang with similar features enabled on cmake so hopefully this time CI will pass.

Cheers.

@@ -56,11 +102,13 @@ GIT_EXTERN(int) git_signature_now(git_signature **out, const char *name, const c
* based on that information. It will return GIT_ENOTFOUND if either the
* user.name or user.email are not set.
*
* @deprecated use git_signature_default_author or git_signature_default_committer instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Let's not deprecate itper se - at least not yet - typically, we would compile that out of builds whenDEPRECATE_HARD is defined. It's usually the thing that we do before its eventual removal. Instead, let's adjust the documentation to suggest thatgit_signature_default_author are probably what peopleactually want. We can deprecate in a future release if we think that's wise.

And it may be - but I think that there are likely a lot of people who are using this API at the moment. So let's give them some time to adjust.

@ethomson
Copy link
Member

A few minor tweaks requested - thanks for putting this together, I think that this is a really useful improvement. 🙏

When creating an action signature (e.g. for a commit author andcommitter) read the following environment variables that can overridethe configuration options: * `GIT_AUTHOR_NAME` is the human-readable name in the "author" field. * `GIT_AUTHOR_EMAIL` is the email for the "author" field. * `GIT_AUTHOR_DATE` is the timestamp used for the "author" field. * `GIT_COMMITTER_NAME` sets the human name for the "committer" field. * `GIT_COMMITTER_EMAIL` is the email address for the "committer" field. * `GIT_COMMITTER_DATE` is used for the timestamp in the "committer"   field. * `EMAIL` is the fallback email address in case the user.email   configuration value isn't set. If this isn't set, Git falls back to   the system user and host names.This is taken from the git documentation chapter "10.8 EnvironmentVariables":https://git-scm.com/book/en/v2/Git-Internals-Environment-VariablesThis PR adds support for reading these environment variables by addingtwo new functions `git_signature_default_author` and`git_signature_default_committer` and deprecates the`git_signature_default` function.Fixes:libgit2#3751Prior work: *libgit2#4409 *libgit2#5479 *libgit2#6290
Also fix some comment formatting.
@u-quark
Copy link
ContributorAuthor

Thanks of the review@ethomson!

@csware
Copy link
Contributor

The usage of environment variables should always be opt-in in libgit2

Making the various pieces that create commits automatically (eg, rebase)start paying attention to the environment variables is a Big Change.For now, this is a big change in defaults; we should treat it asbreaking. We don't move to this by default; we may add `from_env` or`honor_env` type of API surface in the future.
People who are doing a commit expect a unified timestamp betweenauthor and committer information when we're using the current timestamp.Provide a single function that returns both author and committerinformation so that they can have an identical timestamp when none isspecified in the environment.
@ethomson
Copy link
Member

Sorry, I've been slow to look at this. One of the things that I've found is that if you want to emulategit commit (for example), then you really want to a) look at the environment variables, but b) fall back to thesame time for both author and committer if you're using the current timestamp. I think that this needs to be a single API to get both instead of individual APIs.

Per@csware 's comment, I also think that switching all the behavior of the things that produce commits internally to look at the environment variables is a Big Change that would be unexpected in a small release. I would suggest that we keep the current behavior (wherein it does not honor theGIT_AUTHOR_ andGIT_COMMITTER_ environment variables for now. We can potentially make this opt-in (you can imagine a rebase option that says that it should honor the environment variable). We could also change the behavior of libgit2 to start paying attention to environment variables by default in a future release, with an explicit opt-out. But for now, I think that it needs to remain opt-in.

I'm going to push up a commit to this based on this to adjust...

@u-quark
Copy link
ContributorAuthor

Thanks for looking at this@ethomson, I've also been busy and didn't chase this.

I understand your points and they make sense! Both changes LGTM. I will have a closer look later this week and we should be able to merge this.

ethomson reacted with heart emoji

@ethomson
Copy link
Member

Thanks - sorry to bring the chaotic energy and just push a commit on top of your branch. I appreciate you opening this PR and apologize again for the delay.

ZhangLyndon added a commit to ZhangLyndon/libgit2 that referenced this pull requestJul 7, 2024
@ethomson
Copy link
Member

Thanks again for the PR!

@ethomsonethomson merged commitf3518ee intolibgit2:mainJul 10, 2024
36 checks passed
u-quark added a commit to u-quark/gg that referenced this pull requestJul 24, 2024
Withlibgit2/libgit2#6706 merged in we can nowuse libgit2 from master. Small changes were needed.This is needed in order to have stable hashes for e2e tests.Previously we needed to use a patched version from:https://github.com/u-quark/libgit2/commits/gg
@u-quark
Copy link
ContributorAuthor

I just had some time to look into this. This is great! Works for me like a charm!

Thanks for merging this and sorry again for the late response.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ethomsonethomsonethomson requested changes

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Function to parse author and committer information from environment
3 participants
@u-quark@ethomson@csware
[8]ページ先頭
©2009-2025 Movatter.jp