Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebase via PR menu doesn't trigger hook #104

Open
DrVanScott opened this issue Nov 26, 2019 · 5 comments
Open

Rebase via PR menu doesn't trigger hook #104

DrVanScott opened this issue Nov 26, 2019 · 5 comments
Labels

Comments

@DrVanScott
Copy link

In the "three dot" menu of a PR in Bitbucket, it is possible to start a rebase. Hooks are not executed for the new commits which will be pushed

Bitbucket: 6.5.1
Plugin: 7.2.0-1

@seletskiy
Copy link
Member

seletskiy commented Nov 26, 2019

@DrVanScott

Hello. Thanks for the report. We'll check it in our test setup and get back shortly.

@seletskiy
Copy link
Member

According to our research so far it's not yet possible to call any hook scripts on rebase event due internal logic of the Bitbucket Server.

I'm passing this ticket next to Bryan who is working closely on the Bitbucket Server.

@bturner: please correct if I'm wrong on this, but it seems that rebase internally triggers GitPullRequestRebasedEvent which can't be used to register our hook scripts to be triggered by it because it doesn't implement RepositoryHookTrigger interface.

@bturner
Copy link

bturner commented Nov 28, 2019

@seletskiy,

Sure, it's possible. I think you're looking for GitRepositoryHookTrigger.REBASE. The link is to the 6.8.1 Javadocs, but the enum and the REBASE constant have existed since Bitbucket Server 5.5.0, so you can likely support hooks on rebases even prior to the HookScript support we added in 6.2.0.

Hope this helps,
Bryan Turner
Atlassian Bitbucket

@kovetskiy
Copy link
Member

@bturner

There is a nuance that this event is triggered every time when a user opens a pull request page because UI triggers following API to get canRebase and canWrite variables from backend:
/rest/git/latest/projects/{project_key}/repos/{repo_slug}/pull-requests/{pull_request_id}/rebase

As far as I understand DrVanScott, users want to run HookScripts before the rebase operation and right after it while GitRepositoryHookTrigger.REBASE events are triggered for git-rebase checks.

The GitPullRequestRebasedEvent event (which doesn't work for us because it doesn't implement RepositoryHookTrigger) looks more interesting because it goes right from DefaultGitPullRequestService.RebaseOperation immediately after executing git rebase command. (I am looking at BB 6.8.0 sources /com/atlassian/stash/internal/scm/git/pull/DefaultGitPullRequestService.java)

Also, I have created two HookScripts for both types HookScriptType.PRE and HookScriptType.POST and for some reason only HookScriptType.PRE was invoked (the PRE one didn't exit with non-zero code, I double-checked it).

Thanks for your help. We appreciate it.

@bturner
Copy link

bturner commented Nov 28, 2019

@kovetskiy,

Triggering the hook in canRebase is intentional, to allow hook implementors to block rebasing before we try to perform it. When possible, it's much better if hooks can reject the rebase before the system actually runs git rebase, to avoid doing throwaway work. If you don't want to trigger hook scripts from your app in that case, the isDryRun() flag can be used to tell the difference between canRebase and rebase. If you do trigger hook scripts, the scripts themselves can use BB_IS_DRY_RUN to tell the difference (in 6.2+, at least). This same dry-run/non-dry-run approach is taken with StandardRepositoryHookTrigger.PULL_REQUEST_MERGE; hooks are invoked every time canMerge is checked, which is done every time the pull request page is opened. How does your app handle that?

Based on that, @DrVanScott would look for BB_IS_DRY_RUN=false to tell that it's the "real" rebase operation, with hooks being invoked after the rebase is done but before any refs are updated. Since the opening description references "for the new commits", it seems like HookScriptType.PRE handling may be all he really cares about. (He can correct me if I'm wrong, of course.) If his hook script rejects the commits, the rebase results will be discarded without updating any refs--just like any other PRE hook.

The lack of a HookScriptType.POST for rebase is an oversight. There should still be a fallback "generic" event that triggers hooks after the rebase, using the catch-all UNKNOWN trigger. Unfortunately, that UNKNOWN trigger can also be used for other places where the system isn't mapping an event to a specific RepositoryHookTrigger, so pretending like UNKNOWN means REBASE isn't reliable. I'll create a BSERV issue for tracking getting a "real" POST trigger on that.

Best regards,
Bryan Turner
Atlassian Bitbucket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants