@geekdave I had issues with git clone (taking forever) and git merge (failing, asking for git username and email) in the approach I described above. Our current solution that seems to be working so far is:
clone:
git:
image: plugins/git
# We clone submodules ourselves in ci-clone
recursive: false
pipeline:
# We add a custom clone step to workaround a bug with GitHub merge ref
clone:
image: plugins/git
commands:
- ./ci-clone
And ci-clone defined as:
#!/usr/bin/env sh
set -eux
# if build is PR rebase on top of target branch
if [ "$DRONE_BUILD_EVENT" = "pull_request" ]; then
git config user.email "[email protected]"
git config user.name "Some Name"
git pull "$DRONE_REMOTE_URL" "$DRONE_BRANCH"
fi
git submodule update --init --recursive
Also looking at your example, it would only work with intra-repository pull requests, since you would need to use the merge ref if the pull request is coming from a fork.
uses the head ref for pull requests and manually performs the merge
guarantees the correct pull request sha and will fail when encountering merge conflicts
preserves the branch and sha and avoids a detached HEAD state (supporting tools that use git rev-parse HEAD)
There are some limitations with the implementation
pull requests tested with github only; ref logic in place for gitlab, gitea, gogs and bitbucket
does not yet work with github deployment events
does not yet support skip verify
does not yet support non-empty directories (important if caching .git directory)
does not support submodules (this is not planned, cloning submodules should instead be handled as a step in your pipeline)
There is solid unit testing in place and I think this provides a good base that we can build upon. If this is something you would like to help accelerate, please consider donating a few hours of time to help test and provide feedback and ways to improve.
You can use this plugin today for non-critical projects with the following syntax:
Glad to be a guinea pig, @bradrydzewski! I fired up this new image in my clone phase, and got this error right out-of-the gate on the /pr job (/push job ran normally):
chmod: 600: No such file or directory
Initialized empty Git repository in /drone/src/github.com/myorg/dummy-repo/.git/
+ git fetch origin +refs/heads/master:
From https://github.com/myorg/dummy-repo
* branch master -> FETCH_HEAD
* [new branch] master -> origin/master
+ git checkout master
Branch master set up to track remote branch master from origin.
Already on 'master'
+ git fetch origin pull/28/head:
From https://github.com/myorg/dummy-repo
* branch refs/pull/28/head -> FETCH_HEAD
+ git rebase 38482f303c72888a96d50338a5d8b466db9c097d
First, rewinding head to replay your work on top of it...
*** Please tell me who you are.
Run
git config --global user.email "[email protected]"
git config --global user.name "Your Name"
to set your account's default identity.
Omit --global to set the identity only in this repository.
fatal: unable to auto-detect email address (got '[email protected](none)')
Here’s my drone.yml:
clone:
git:
image: plugins/git:next
pipeline:
canary:
image: alpine
commands:
- echo "This is a test to make sure Drone is running jobs."
I added a default email and name (via environment variable) which should satisfy that github prompt if it appears again. A new image is now available. Note that I didn’t have a test PR to reproduce the prompt, so this is untested, but according to git docs it should work.
Thanks - I’ll try this. Since I already have the :next version installed on several agent machines, is there an easy way to force the upgrade without manually removing that image from each machine?
First of all, big thanks for this. Outdated merge heads from Github started to pop up more and more in our deployment.
We now started using this in a few repo. First executions look good, and we are happy that this is hash based now.
There was one case though that may be of interest: in one of our long running branches the rebase failed with a conflict, while a manual merge of master to this branch succeeded without conflict. Retrying the build after this merge, the rebase succeeded as nothing left to rebase really.
I’m thinking that git rebase <hash> is perhaps more fragile than git reset --hard <hash> followed by a git merge master. But I’m sure you have reasons why it’s a rebase and not a merge.
not really, it was just the first thing that passed unit tests
The primary goal was was to preserve the hash and not create a new merge commit or create a detached head state. If the unit tests pass without modification with reset --hard I am open to adjusting. Thanks for testing, and for the feedback!
Just wanted to verify that we’ll be able to continue to pull the specific branch/commit without doing any of the auto-rebasing stuff. In our case, the extra magic hurts us more than it helps.
@gtaylor I recommend either testing the shell script locally, or maybe even testing with one of your repositories if possible. If we identify issues over the next few weeks, it will give us time to fix before this becomes the default in 0.9
With those criteria I couldn’t come up with an alternative. My original proposal creates both a detached head (since we don’t know the source branch) and a merge commit. So I stick with what’s there already. Thanks.
It’s very minimal feedback Brad, but “next” is not picking up the fact we’re pulling from GitLab and changing the ref path. (Drone : 0.8.2). I assume that’s because the $DRONE_COMMIT_REF isn’t being evaluated correctly - but I’m not sure how to make that obvious and get that information for you. Let me know if I can be of any help.
Update: Here’s what I get from my build (and I don’t know why it doesn’t pick it up in “next”)
echo $DRONE_COMMIT_REF
refs/merge-requests/1/head
Update: Seems it’s down to the version of /bin/sh. I’ve submitted a PR with a workaround.
I tried using git:next and it seems that cd to $DRONE_WORKSPACE fails since the directory does not exist.
On the other hand, it seems that code in master branch creates the workspace directory if it does not exist.
It would be helpful if you could add the same procedure (create the workspace directory) to next branch.
Here is a data point - the GitHub refspecs have been particularly flaky today, meaning that Drone was giving very confusing errors for hours. I ended up switching that repo to plugins/git:next, and the problems disappeared immediately. We’re going to merge it into master, as I don’t see any regression or bug in that version
We’ve been using it and it has been great. I would love to see it merged because it is a pain to have to add it. When I forget it we have issues on new projects.
@bradrydzewski Thanks for working on improving the cloning process! We are running into issues with drone building non-current versions of PRs on a weekly basis.
Having said that, I would like to suggest a) using git merge 26923... instead of git rebase 26923... since this more closely resembles what happens on Github when you merge a PR. Also, there can be merge conflicts for a rebase that do not exist for a merge.
And b) the whole idea of “we merge ourselves because Github is too slow” is not really implemented currently, since with git fetch origin pull/14596/head: we still depend on the Github pull ref to be up-to-date. And because I see now way of avoiding to use the Github pull ref for inter-repository PRs, I would suggest to simply check that the fetched ref matches the DRONE_COMMIT_SHA and retry if not (yet).
If these ideas are welcome, I would volunteer to work on a PR for drone-git:next for this!?
Or should it be a PR for drone-git:master for only my idea b)?
Were there other reasons for next except outdated Github pull refs? (Yes, I have seen the listed benefits at the top post, but… 1) is true but maybe not so relevant since github itself tells you something about merge failures already, 2) is also in my idea b), 3) is not possible for inter-repo PRs and 4) I do not really understand - since the head sha will be changed either way (true for rebase, reset and merge)
