mirror of
https://github.com/facebook/sapling.git
synced 2024-10-06 23:07:18 +03:00
move git commit-tree calls behind a common API
Summary: We need to consolidate this logic so that for Sapling: - We ensure `user.name` and `user.email` are set. - We can specify the appropriate GPG flags. Reviewed By: zzl0 Differential Revision: D41791311 fbshipit-source-id: c02722e72caa1078fdfe7438dbfaad51ed040717
This commit is contained in:
Michael Bolin
Facebook GitHub Bot
parent
1deaf2a79d
commit
4f93e85e9d
@ -1,37 +0,0 @@
|
||||
"""This module determines if the commits need to be signed.
|
||||
We need to do this manually, because ghstack uses commit-tree instead of commit.
|
||||
commit-tree command doesn't pick up commit.gpgsign git config
|
||||
|
||||
The porcelain git behavior w.r.t. signing is
|
||||
|
||||
when both `commit.gpgsign` and `user.signingkey` are set, the commit is signed
|
||||
when only `commit.gpgsign` is true, git errors out
|
||||
|
||||
This module will retain this behavior:
|
||||
We will attempt to sign as long as `commit.gpgsign` is true.
|
||||
If not key is configure, error will occur
|
||||
"""
|
||||
import subprocess
|
||||
from typing import Tuple, Union
|
||||
|
||||
import ghstack.shell
|
||||
|
||||
_should_sign = None
|
||||
|
||||
|
||||
def gpg_args_if_necessary(
|
||||
shell: ghstack.shell.Shell = ghstack.shell.Shell()
|
||||
) -> Union[Tuple[str], Tuple[()]]:
|
||||
global _should_sign
|
||||
# cache the config result
|
||||
if _should_sign is None:
|
||||
# If the config is not set, we get exit 1
|
||||
try:
|
||||
# Why the complicated compare
|
||||
# https://git-scm.com/docs/git-config#Documentation/git-config.txt-boolean
|
||||
_should_sign = shell.git("config", "--get", "commit.gpgsign") in ("yes", "on", "true", "1")
|
||||
except (subprocess.CalledProcessError, RuntimeError):
|
||||
# Note shell.git() raises RuntimeError for a non-zero exit code.
|
||||
_should_sign = False
|
||||
|
||||
return ("-S",) if _should_sign else ()
|
||||
@ -4,9 +4,11 @@ import os
|
||||
import shlex
|
||||
import subprocess
|
||||
import sys
|
||||
from typing import (IO, Any, Dict, Optional, Sequence, Tuple, TypeVar, Union,
|
||||
from typing import (IO, Any, Dict, List, Optional, Sequence, Tuple, TypeVar, Union,
|
||||
overload)
|
||||
|
||||
from ghstack.ghs_types import GitCommitHash
|
||||
|
||||
# Shell commands generally return str, but with exitcode=True
|
||||
# they return a bool, and if stdout is piped straight to sys.stdout
|
||||
# they return None.
|
||||
@ -281,6 +283,18 @@ class Shell(object):
|
||||
|
||||
return self._maybe_rstrip(self.sh(*(("git",) + args), **kwargs))
|
||||
|
||||
def git_commit_tree(self, *args, **kwargs: Any # noqa: F811
|
||||
) -> GitCommitHash:
|
||||
"""Run `git commit-tree`, adding GPG flags, if appropriate.
|
||||
"""
|
||||
gpg_args = self.get_gpg_args()
|
||||
full_args = ["commit-tree"] + gpg_args + list(args)
|
||||
return GitCommitHash(self.git(*full_args, **kwargs))
|
||||
|
||||
def get_gpg_args(self) -> List[str]:
|
||||
"""args to include with `git commit` or `git commit-tree` for GPG signing"""
|
||||
return gpg_args_if_necessary(self)
|
||||
|
||||
@overload # noqa: F811
|
||||
def hg(self, *args: str) -> str:
|
||||
...
|
||||
@ -343,3 +357,22 @@ class Shell(object):
|
||||
d: directory to change to
|
||||
"""
|
||||
self.cwd = os.path.join(self.cwd, d)
|
||||
|
||||
|
||||
_should_sign = None
|
||||
|
||||
|
||||
def gpg_args_if_necessary(shell: Shell) -> List[str]:
|
||||
global _should_sign
|
||||
# cache the config result
|
||||
if _should_sign is None:
|
||||
# If the config is not set, we get exit 1
|
||||
try:
|
||||
# Why the complicated compare
|
||||
# https://git-scm.com/docs/git-config#Documentation/git-config.txt-boolean
|
||||
_should_sign = shell.git("config", "--get", "commit.gpgsign") in ("yes", "on", "true", "1")
|
||||
except (subprocess.CalledProcessError, RuntimeError):
|
||||
# Note shell.git() raises RuntimeError for a non-zero exit code.
|
||||
_should_sign = False
|
||||
|
||||
return ["-S"] if _should_sign else []
|
||||
|
||||
@ -9,7 +9,6 @@ import ghstack.diff
|
||||
import ghstack.git
|
||||
import ghstack.github
|
||||
import ghstack.github_utils
|
||||
import ghstack.gpg_sign
|
||||
import ghstack.logs
|
||||
import ghstack.shell
|
||||
from ghstack.ghs_types import (GhNumber, GitCommitHash, GitHubNumber,
|
||||
@ -587,10 +586,8 @@ Since we cannot proceed, ghstack will abort now.
|
||||
assert ghnum not in self.seen_ghnums, f"ghnum {ghnum} already seen"
|
||||
self.seen_ghnums.add(ghnum)
|
||||
|
||||
new_pull = GitCommitHash(
|
||||
self.sh.git("commit-tree", *ghstack.gpg_sign.gpg_args_if_necessary(self.sh),
|
||||
"-p", self.base_commit, tree,
|
||||
input=commit.summary + "\n\n[ghstack-poisoned]"))
|
||||
new_pull = self.sh.git_commit_tree("-p", self.base_commit, tree,
|
||||
input=commit.summary + "\n\n[ghstack-poisoned]")
|
||||
|
||||
# Push the branches, so that we can create a PR for them
|
||||
new_branches = (
|
||||
@ -795,15 +792,14 @@ Since we cannot proceed, ghstack will abort now.
|
||||
# the fact that we still incorrectly report
|
||||
# the old base as an ancestor of our commit, but
|
||||
# it's better than nothing.
|
||||
new_base = GitCommitHash(self.sh.git(
|
||||
"commit-tree", *ghstack.gpg_sign.gpg_args_if_necessary(self.sh),
|
||||
new_base = self.sh.git_commit_tree(
|
||||
"-p",
|
||||
orig_base_hash,
|
||||
*(() if same_stack_base else ("-p", self.stack_base)),
|
||||
self.base_tree,
|
||||
input='Update base for {} on "{}"\n\n{}\n\n[ghstack-poisoned]'
|
||||
.format(self.msg, elab_commit.title,
|
||||
non_orig_commit_msg)))
|
||||
non_orig_commit_msg))
|
||||
|
||||
base_args = ("-p", new_base)
|
||||
|
||||
@ -824,12 +820,11 @@ Since we cannot proceed, ghstack will abort now.
|
||||
repo_id=self.repo_id,
|
||||
ref=branch_head(username, ghnum)
|
||||
)['commit']
|
||||
new_pull = GitCommitHash(self.sh.git(
|
||||
"commit-tree", *ghstack.gpg_sign.gpg_args_if_necessary(self.sh),
|
||||
new_pull = self.sh.git_commit_tree(
|
||||
"-p", head_hash,
|
||||
*base_args,
|
||||
tree,
|
||||
input='{} on "{}"\n\n{}\n\n[ghstack-poisoned]'.format(self.msg, elab_commit.title, non_orig_commit_msg)))
|
||||
input='{} on "{}"\n\n{}\n\n[ghstack-poisoned]'.format(self.msg, elab_commit.title, non_orig_commit_msg))
|
||||
|
||||
# Perform what is effectively an interactive rebase
|
||||
# on the orig branch.
|
||||
@ -920,12 +915,7 @@ Since we cannot proceed, ghstack will abort now.
|
||||
# TODO: Try harder to preserve the old author/commit
|
||||
# information (is it really necessary? Check what
|
||||
# --amend does...)
|
||||
return GitCommitHash(self.sh.git(
|
||||
"commit-tree",
|
||||
*ghstack.gpg_sign.gpg_args_if_necessary(self.sh),
|
||||
"-p", self.base_orig,
|
||||
tree,
|
||||
input=commit_msg))
|
||||
return self.sh.git_commit_tree("-p", self.base_orig, tree, input=commit_msg)
|
||||
|
||||
def _format_stack(self, index: int) -> str:
|
||||
rows = []
|
||||
|
||||
@ -8,7 +8,6 @@ import ghstack.diff
|
||||
import ghstack.git
|
||||
import ghstack.github
|
||||
import ghstack.github_utils
|
||||
import ghstack.gpg_sign
|
||||
import ghstack.shell
|
||||
from ghstack.ghs_types import GitCommitHash, GitTreeHash
|
||||
|
||||
@ -128,12 +127,10 @@ def main(*,
|
||||
# it so the return value is consistent with the Git codepath.
|
||||
head = GitCommitHash(stack[index].commit_id)
|
||||
else:
|
||||
head = GitCommitHash(sh.git(
|
||||
"commit-tree",
|
||||
*ghstack.gpg_sign.gpg_args_if_necessary(sh),
|
||||
head = sh.git_commit_tree(
|
||||
s.tree,
|
||||
"-p", head,
|
||||
input=commit_msg))
|
||||
input=commit_msg)
|
||||
|
||||
if sh.is_git():
|
||||
sh.git('reset', '--soft', head)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user