greenkeeperio / greenkeeper-lockfile

:lock: Your lockfile, up to date, all the time
https://greenkeeper.io
183 stars 73 forks source link

Lockfile update being skipped with "Only running on first push of a new branch" even though there's only one commit #100

Open rsimha opened 6 years ago

rsimha commented 6 years ago

We recently enabled greenkeeper and greenkeeper-lockfile for our project at github.com/ampproject/amphtml, which uses yarn for package management, and therefore, contains the package.json and yarn.lock files.

We now have a handful of PRs opened by the greenkeeper bot with updates to package.json, but with no updates to yarn.lock. I made sure we were setting a GH_TOKEN and then rebased the greenkeeper/* branches on the latest HEAD on our master branch in the hope that the lockfiles are updated with the new GH_TOKEN present.

When I look at the Travis CI logs, I see that greenkeeper-lockfile-update is returning the status Only running on first push of a new branch in spite of the fact that the branch has only one commit pushed to it. As a result, we cannot merge any of the PRs created by greenkeeper.

Any idea what's going on here, and what I can do to make sure greenkeeper-lockfile-update actually updates the lockfile?

Here's an example of a PR: https://github.com/ampproject/amphtml/pull/12793 And here are the corresponding Travis CI logs: https://travis-ci.org/ampproject/amphtml/jobs/327981188#L673

Alternatively, if these PRs are a lost cause due to the "first push" rule being violated, would it help to delete the greenkeeper branches so that they can be freshly generated by the bot in due course of time and have the lockfile properly updated?

Thanks!

keimlink commented 6 years ago

We're having the same problem. Builds triggered by Greenkeeper that should update yarn.lock don't do it. We're seeing the same error message:

This build should not update the lockfile. It could be a PR, not a branch build.

I suppose this caused by the way info.correctBuild is computed. In greenkeeper-lockfile/ci-services/circleci.js info.correctBuild is only true if no pull request exists.

But Greenkeeper created a pull request before the CircleCI job running yarn greenkeeper-lockfile-update had started:

2018-01-18T22:18:17Z: Commit keimlink/docker-sphinx-doc@f3e344f50bef96d79109a05e4040a70ab16c729f was created. 2018-01-18T22:18:30Z: CircleCI workflow keimlink/docker-sphinx-doc #487 - CircleCI started. 2018-01-18T22:18:51Z: Pull request keimlink/docker-sphinx-doc#10 was created. 2018-01-18T22:19:09Z: The CircleCI job that ran yarn greenkeeper-lockfile-update finished. It took only four seconds, so it was started after the pull request had been created. Looking at CI_PULL_REQUEST in "Spin up Environment" confirms this:

CI_PULL_REQUEST=https://github.com/keimlink/docker-sphinx-doc/pull/10

I don't know if TravisCi behaves in the same way. But for CircleCI 2.0 with the new workflows there seems to be a problem with jobs being spawned after the pull request has been created.

Unfortunately I don't have a proposal to fix this issue.

keimlink commented 6 years ago

Amendment: Projects using CircleCI 1.0 with a single machine running the complete build still working fine. 😃

keimlink commented 6 years ago

Update: A build in keimlink/docker-sphinx-doc was started before the pull request has been created.

This build failed too, but with a different error message:

Only running on first push of a new branch

This time the reason seems to be that the environment variable CIRCLE_PREVIOUS_BUILD_NUM was not empty.

Because we use a CircleCI 2.0 workflow other jobs have been executed before. So the environment variable is not empty as it used to be before. It contains the number of the previous job in the workflow.

There are several environment variables only availble in a CircleCI 2.0 workflow:

See also CircleCI Environment Variable Descriptions.

Maybe these can be used to determine if a lockfile update is valid?

keimlink commented 6 years ago

@boennemann What do you think about using log() from steveukx/git-js to check if the lock file has already been updated on the current branch? This could be a more reliable solution then checking environment variables.

Realtin commented 6 years ago

@keimlink I know you have switched away from greenkeeper but I'm still trying to solve this.

I just saw we already have something that counts how many commits there are on a branch: firstPush: gitHelpers.getNumberOfCommitsOnBranch(branchName) === 1 this is used for some of the CI services. Maybe this is a better check then !env.CIRCLE_PREVIOUS_BUILD_NUM as well as !env.TRAVIS_COMMIT_RANGE,

mrhyde commented 6 years ago

@Realtin what you suggest was actually removed in old PR https://github.com/greenkeeperio/greenkeeper-lockfile/pull/32 We are also using Circle CI 2.0 Workflows and the issue with "Only running on first push of a new branch" looks weird and out of place.

Unless someone can explain to me why this type of check is present I would just say we should check for Circle CI 2.0 specific env variables and ditch that error (updating docs will be required of course)

lddubeau commented 6 years ago

I've run into this problem too. Here is my current working hypothesis as to what's happening.

greenkeeper-lockfile determines whether the branch contains a single push by checking if this expression is true: !env.TRAVIS_COMMIT_RANGE. The problem is that there are some conditions under which the value of TRAVIS_COMMIT_RANGE is does not mean what we may think it means. When a branch is initially created, it does track the the range of commits of the new branch. However, rebasing tosses this out the window. Suppose a tree like:

A - B - C - D
     \
      G

Where G is a commit on a Greenkeeper branch that was created from B, but after the branch was made, more commits (C, D) were added to the main branch. The TRAVIS_COMMIT_RANGE for G would be unset because there is only one commit in the branch. Good.

The developers rebase the Greenkeeper branch to incorporate the latest changes and have CI run over the updated code:

A - B - C - D
             \
              G'

If you check the rebased branch on Github, you'll see a single commit (G') as part of the branch. However, Travis will still takes B as the branch's origin, and will use B as the start of the TRAVIS_COMMIT_RANGE. So TRAVIS_COMMIT_RANGE will have a value, which shows the commits from B to G'. It will look like the branch has multiple commits, even though it really has only one.

I've found one Travis issue related to the behavior of TRAVIS_COMMIT_RANGE I describe above.