Skip to content

Add optional push-before-diff check for Git branches in arc diff #289

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
nico97118 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Add optional push-before-diff check for Git branches in arc diff #289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
267 changes: 267 additions & 0 deletions src/workflow/ArcanistDiffWorkflow.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,44 @@ public function getArguments() {
'hg' => pht('Mercurial does not support %s yet.', '--head'),
),
),
'no-push-before-diff' => array(
'paramtype' => 'bool',
'help' => pht(
'If set, override .arcconfig and disable push-before-diff checks '.
'Overrides arc.diff.push-before-diff.enabled. '
),
'supports' => array('git'),
'conflicts' => array(
'push-before-diff' => null,
),
),
'push-before-diff' => array(
'paramtype' => 'bool',
'help' => pht(
'If set, require the current branch to be pushed to the remote before creating a diff. '.
'Overrides arc.diff.push-before-diff.enabled. '
),
'supports' => array('git'),
),
'push-before-diff-remote' => array(
'param' => 'remote',
'paramtype' => 'string',
'help' => pht(
'Specify the remote to check for pushed commits before creating a diff. '.
'Overrides arc.diff.push-before-diff.remote. '.
"Default : 'origin'."),
'supports' => array('git'),
),
'push-before-diff-mode' => array(
'param' => 'mode',
'paramtype' => 'string',
'help' => pht(
'Select behavior if the branch is not pushed before diff: '.
'"error" (fail) or "warn" (print warning, continue). '.
'Overrides arc.diff.push-before-diff.mode. '.
"Default : 'error'."),
'supports' => array('git'),
),
);

return $arguments;
Expand All @@ -348,6 +386,8 @@ public function run() {
$this->runRepositoryAPISetup();
$this->runDiffSetupBasics();

$this->validateBranchPushed();

$commit_message = $this->buildCommitMessage();

$this->dispatchEvent(
Expand Down Expand Up @@ -2897,4 +2937,231 @@ private function getDependsOnRevisionRef() {
return head($revision_refs);
}

/**
* Verify that the current Git branch is pushed to the expected remote before
* creating a diff.
*
* This check enforces that the local branch has an upstream configured for the
* remote defined by Arcanist (via CLI arguments or .arcconfig), and that:
*
* - an upstream branch is explicitly configured for the current branch
* - the upstream targets the expected remote
* - the upstream branch exists on the remote
* - all local commits are present on the upstream branch
*
* The behavior on failure (error or warning) is controlled by the
* `push-before-diff.mode` setting.
*/
private function validateBranchPushed() {
// Arguments CLI → .arcconfig → valeurs par défaut
if ($this->getArgument('no-push-before-diff')) {
return;
}
$require_pushed = $this->getArgument('push-before-diff');
if ($require_pushed === null) {
$require_pushed = $this->getConfigFromAnySource(
'arc.diff.push-before-diff.enabled',
false
);
}

if (!$require_pushed) {
return;
}

$remote = $this->getArgument('push-before-diff-remote');
if ($remote === null) {
$remote = $this->getConfigFromAnySource(
'arc.diff.push-before-diff.remote',
'origin'
);
}

$mode = $this->getArgument('push-before-diff-mode');
if ($mode === null) {
$mode = $this->getConfigFromAnySource(
'arc.diff.push-before-diff.mode',
'error'
);
}
if (!in_array($mode, array('error', 'warn'), true)) {
throw new ArcanistUsageException(
pht(
'Invalid value for --push-before-diff-mode: %s. Expected "error" or "warn".',
$mode
)
);
}

$repository = $this->getRepositoryAPI();
if (!$repository instanceof ArcanistGitAPI) {
// Only supported for Git repositories.
return;
}

// Determine current local branch
$local_branch = $repository->getBranchName();
if (!$local_branch) {
$this->handleRequirePushedFailure(
$mode,
pht(
"You are in a detached HEAD state.\n".
"Unable to verify if commits are pushed."
)
);
return;
}

// Read upstream configuration for the local branch
try {
$configured_remote = trim($repository->execxLocal(
'config --get branch.%s.remote',
$local_branch
)[0]);
$configured_merge = trim($repository->execxLocal(
'config --get branch.%s.merge',
$local_branch
)[0]);
} catch (Exception $ex) {
$this->handleRequirePushedFailure(
$mode,
pht(
"No upstream branch is configured for branch \"%s\" on remote \"%s\".\n".
"HINT: You may need to push your branch with:\n".
" git push -u %s %s",
$local_branch,
$remote,
$remote,
$local_branch
)
);
return;
}

// Ensure upstream is configured for the expected remote
if ($configured_remote !== $remote) {
$this->handleRequirePushedFailure(
$mode,
pht(
"Branch \"%s\" is configured to push to remote \"%s\", but this workflow requires it to be pushed to \"%s\".",
$local_branch,
$configured_remote,
$remote
)
);
return;
}

// Extract upstream branch name from refs/heads/<branch>
if (!preg_match('#^refs/heads/(.+)$#', $configured_merge, $matches)) {
$this->handleRequirePushedFailure(
$mode,
pht(
"Unable to determine upstream branch name from merge ref \"%s\".",
$configured_merge
)
);
return;
}

$upstream_branch = $matches[1];

// Verify that the upstream branch exists on the remote
try {
$repository->execxLocal(
'show-ref --verify --quiet refs/remotes/%s/%s',
$remote,
$upstream_branch
);
} catch (Exception $ex) {
$this->handleRequirePushedFailure(
$mode,
pht(
"Upstream branch \"%s/%s\" does not exist on the remote.\n".
"HINT: You may need to push your branch with:\n".
" git push -u %s %s",
$remote,
$upstream_branch,
$remote,
$upstream_branch
)
);
return;
}

// Verify that there are no local commits missing from the upstream branch
list($ahead) = $repository->execxLocal(
'rev-list --count %s/%s..HEAD',
$remote,
$upstream_branch
);

if ((int)$ahead > 0) {
$this->handleRequirePushedFailure(
$mode,
pht(
"%s contains commits which have not been pushed.\n".
"HINT: You can push your commits with:\n".
" git push %s %s\n",
$local_branch,
$remote,
$local_branch
)
);
return;
}
$this->writeInfo(
pht('Push-before-diff'),
pht(
'All commits on branch "%s" have been pushed to "%s/%s".',
$local_branch,
$remote,
$upstream_branch
)
);
}

/**
* Handle a branch push validation failure according to the configured mode.
*
* This function is called when `validateBranchPushed()` detects that the
* current Git branch is not fully pushed to the expected remote.
*
* Behavior depends on the `$mode` argument:
*
* - 'warn': Display a warning message and prompt the user to confirm whether
* to continue despite the branch not being fully pushed.
* - 'error': Immediately abort the workflow by throwing an exception with
* the provided message.
*
* @param string $mode The failure handling mode ('warn' or 'error').
* @param string $message A human-readable message explaining the validation failure.
*
* @throws ArcanistUsageException When the mode is 'error' or the user cancels
* after a warning prompt.
*/
private function handleRequirePushedFailure($mode, $message) {
if ($mode === 'warn') {
$this->writeWarn('Push-before-diff', true);
printf(trim($message));

$confirmed = phutil_console_confirm(
"Branch not fully pushed. Continue anyway?",
$default_no = true
);

if (!$confirmed) {
throw new ArcanistUsageException(
"Operation aborted by user because branch is not pushed."
);
}

} else {
// Append bypass hint
$message .= "\n\n".pht("You can bypass this check with '--no-push-before-diff'.");
throw new ArcanistUsageException($message);
}
}

}