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

Checkout Sha when building Pull Request #1000

Closed
os12 opened this issue Apr 30, 2015 · 4 comments
Closed

Checkout Sha when building Pull Request #1000

os12 opened this issue Apr 30, 2015 · 4 comments

Comments

@os12
Copy link

os12 commented Apr 30, 2015

Consider the following scenario:

  1. a PR is posted
    • it schedules the first build
  2. a few more commits are pushed
    • each schedules a build

There appears to be a race when step (2) is repeated several times quickly. Consider the following Drone output:

Building 7164e168f7

git checkout -qf -b pr/415 origin/pr/415
git log .....

Building the following revisions:

* af4fa16 (HEAD, origin/pr/415, pr/415) CI: imposed a timeout on the tests
* 7164e16 CI: dump git revisions
* d040da1 CI: toned down compilation output
* 847bdca CI: log some info
| *   743e868 (origin/master, origin/HEAD, master) Merge pull request #413 from kp6/nbd
| |\  
|/ /  
| * d7cd328 test/harness: nbd::Client adapter
| * d2f24fd untabified
* |   8f376db Merge pull request #414 from os12/master
|\ \  
| |/  
|/|   
| * 81d7b5b block/Client: corrected the special "in flight" case
| * 0fd701d block/Client: minor syntactic cleanups
|/  
*   285a4c8 Merge pull request #410 from os12/rebuild
|\  
| * 1d63993 block/Client: reduced the public API a bit (review)
| * 913b488 tests/perf: corrected mutation tracking for the *Any() case

The build was scheduled to build 7164e168f7 yet it blindly pulled the whole PR. It is actually building af4fa16, which should be built in the next build only.

@os12
Copy link
Author

os12 commented May 1, 2015

I believe the build process needs to change to the specific revision after checking/branching:

git checkout $revision

@bradrydzewski bradrydzewski added this to the v0.4.1 milestone Aug 18, 2015
@bradrydzewski bradrydzewski changed the title A race in the build process when expanding a PR Checkout Sha when building Pull Request Oct 26, 2015
@bradrydzewski bradrydzewski modified the milestones: Unplanned, v0.4.1 Oct 26, 2015
@donatj
Copy link
Contributor

donatj commented Apr 15, 2016

I've got the same issue - quoting my gitter comment:

I pushed two commits, the first one broken, and the second one fixed, maybe 30 seconds apart. I have two matrix builds. The first commits first matrix build was still executing when I pushed the second commit. The second matrix build of the first commit pulled the wrong code - the second, fixed commit, and passed.

Looking at what it executes, it makes sense that it pulled the wrong code. It pulled the FETCH_HEAD which would be ahead of the commit we're analyzing and would be the current commit on that PR.

$ git fetch --no-tags --depth=50 origin +refs/pull/3385/merge:
From https://github.com/redacted/redacted
 * branch            refs/pull/3385/merge -> FETCH_HEAD
$ git checkout -qf FETCH_HEAD

@bradrydzewski
Copy link
Contributor

bradrydzewski commented Apr 15, 2016

This isn't something I'll be focusing on in the near term so I'm marking this issue as help wanted. The default behavior can be changed in the git clone plugin to checkout the SHA instead of FETCH_HEAD
https://github.com/drone-plugins/drone-git/blob/master/main.go#L140

@harness harness locked and limited conversation to collaborators Apr 15, 2016
@bradrydzewski
Copy link
Contributor

closing this because drone core does not decide how a pull request is cloned. It is decided by the plugin, which in this case is the git clone plugin. Feel free to open an issue or send a pull request to the git clone plugin, or even fork the plugin to use your own customized clone logic
https://github.com/drone-plugins/drone-git

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

No branches or pull requests

3 participants