Skip to content

Comments

builder: Ensure committer is set when running git-commit#183

Closed
hadess wants to merge 1 commit intomasterfrom
wip/hadess/git-init-committer
Closed

builder: Ensure committer is set when running git-commit#183
hadess wants to merge 1 commit intomasterfrom
wip/hadess/git-init-committer

Conversation

@hadess
Copy link
Contributor

@hadess hadess commented Jul 13, 2018

Otherwise buildbots, and other non-interactive/clean-room builds might
not have git committer information set, and make it impossible to
successfully commit, and git erroring out with:
*** Please tell me who you are.

This should fix flathub/flathub#443 (comment)

@hadess hadess force-pushed the wip/hadess/git-init-committer branch from 2621f4d to 264e8c8 Compare July 13, 2018 15:02
@hadess
Copy link
Contributor Author

hadess commented Jul 13, 2018

Fully tested, but uglier than expected.

email = trim_linefeed (email);
g_setenv ("GIT_AUTHOR_EMAIL", email, FALSE);
g_setenv ("GIT_COMMITTER_EMAIL", email, FALSE);
g_free (email);
Copy link
Member

Choose a reason for hiding this comment

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

Overriding the environment globally like this feels a bit weird, as it will affect all processes that f-b later spawns.
Either we should pass the env vars specifically only to the child, or we should do this entire dance up-front in main() so that it affect all git calls.

@alexlarsson
Copy link
Member

I think just forcing GIT_AUTHOR_EMAIL and GIT_COMMITTER_EMAIL to be set in main() is the right approach. Then we're guaranteeing this to work for all git invocations, and we make the result more reproducible than sometimes using the real authors name. No created commits from flatpak-builder are persistent, except possibly flatpak-builder --run, so we should not set it there.

@hadess
Copy link
Contributor Author

hadess commented Aug 8, 2018

I think just forcing GIT_AUTHOR_EMAIL and GIT_COMMITTER_EMAIL to be set in main() is the right approach. Then we're guaranteeing this to work for all git invocations, and we make the result more reproducible than sometimes using the real authors name. No created commits from flatpak-builder are persistent, except possibly flatpak-builder --run, so we should not set it there.

If I do that, we don't have a GIT_AUTHOR_NAME that corresponds to the package name, but that's not really a big deal.

@hadess hadess force-pushed the wip/hadess/git-init-committer branch from 264e8c8 to fd5da4d Compare August 8, 2018 10:39
@alexlarsson
Copy link
Member

Yeah, that doesn't seem as important.

Otherwise buildbots, and other non-interactive/clean-room builds might
not have git committer information set, and make it impossible to
successfully commit, and git erroring out with:
*** Please tell me who you are.
@hadess hadess force-pushed the wip/hadess/git-init-committer branch from fd5da4d to 67b5619 Compare August 11, 2018 00:25
@alexlarsson
Copy link
Member

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit 67b5619 has been approved by alexlarsson

@rh-atomic-bot
Copy link

⚡ Test exempted: pull fully rebased and already tested.

@alexlarsson alexlarsson deleted the wip/hadess/git-init-committer branch May 6, 2019 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants