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

feat(ci): avoid push to Azure if branch has been updated #2048

Merged
merged 9 commits into from
Feb 27, 2020

Conversation

robertylewis
Copy link
Member

On non-master builds, it seems unnecessary to push an olean archive if another commit is added to the branch before the build finishes.

A common situation in an open PR is to add a sequence of commits very quickly, e.g. accepting review suggestions. It seems very unlikely anyone would check out one of the commits in the middle.

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • for tactics:
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

@jcommelin
Copy link
Member

+1 from me

@robertylewis robertylewis changed the title avoid push to Azure if branch has been updated feat(ci): avoid push to Azure if branch has been updated Feb 24, 2020
@robertylewis
Copy link
Member Author

I have no idea why this is failing? The new test works when it's performed before the build stage, but not after. See the difference between the last two commits.

@gebner
Copy link
Member

gebner commented Feb 25, 2020

@robertylewis Do you remember the magic number of 60 minutes? I think you need to use the credentials for @leanprover-community-bot.

@robertylewis
Copy link
Member Author

@robertylewis Do you remember the magic number of 60 minutes? I think you need to use the credentials for @leanprover-community-bot.

I think I blocked it out of my memory...

scripts/deploy_nightly.sh Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@robertylewis
Copy link
Member Author

GitHub is struggling right now but I think the fixes made it here.

@robertylewis robertylewis added the awaiting-review The author would like community review of the PR label Feb 25, 2020
@semorrison semorrison added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Feb 27, 2020
@mergify mergify bot merged commit 0e4eb09 into master Feb 27, 2020
@mergify mergify bot deleted the avoid-azure-push branch February 27, 2020 22:05
mergify bot pushed a commit that referenced this pull request Apr 4, 2020
PR #2048 changed the CI so that olean caches are not pushed to Azure if a later commit on the same branch occurs. The reasoning was that it was unlikely that anyone would fetch those caches. That's changed as of #2278, since `fetch_olean_cache.sh` now looks for caches from commits earlier in the history. This means that currently, we can observe the following wasteful sequence of events in several PRs (e.g. #2258):

1. commit A gets pushed to a certain branch and CI_A starts.
2. While CI_A is still running the `leanpkg build` step, commit B is pushed and CI_B starts.
3. CI_A finishes `leanpkg build` but doesn't upload its cache to Azure because of #2048 
4. Now commit C is pushed and can't use the results of CI_A
5. CI_B finishes `leanpkg build` but doesn't upload its cache
6. Commit D is pushed and can't use the results of CI_A or CI_B ...

(This can keep happening as long as the next commit arrives before the most recent CI uploads to Azure).

I also cleaned up some stuff that was left over from when we ran the build on both "pr" and "push" events.
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…community#2048)

* avoid push to Azure if branch has been updated

* changes to git config in deploy_nightly.sh break git fetch

* I don't understand why this is different than on my own repo

* artifact upload breaks fetch, I guess?

* factor out git config

* fix env variable

* adjustments

* removed repo check

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
PR leanprover-community#2048 changed the CI so that olean caches are not pushed to Azure if a later commit on the same branch occurs. The reasoning was that it was unlikely that anyone would fetch those caches. That's changed as of leanprover-community#2278, since `fetch_olean_cache.sh` now looks for caches from commits earlier in the history. This means that currently, we can observe the following wasteful sequence of events in several PRs (e.g. leanprover-community#2258):

1. commit A gets pushed to a certain branch and CI_A starts.
2. While CI_A is still running the `leanpkg build` step, commit B is pushed and CI_B starts.
3. CI_A finishes `leanpkg build` but doesn't upload its cache to Azure because of leanprover-community#2048 
4. Now commit C is pushed and can't use the results of CI_A
5. CI_B finishes `leanpkg build` but doesn't upload its cache
6. Commit D is pushed and can't use the results of CI_A or CI_B ...

(This can keep happening as long as the next commit arrives before the most recent CI uploads to Azure).

I also cleaned up some stuff that was left over from when we ran the build on both "pr" and "push" events.
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…community#2048)

* avoid push to Azure if branch has been updated

* changes to git config in deploy_nightly.sh break git fetch

* I don't understand why this is different than on my own repo

* artifact upload breaks fetch, I guess?

* factor out git config

* fix env variable

* adjustments

* removed repo check

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
PR leanprover-community#2048 changed the CI so that olean caches are not pushed to Azure if a later commit on the same branch occurs. The reasoning was that it was unlikely that anyone would fetch those caches. That's changed as of leanprover-community#2278, since `fetch_olean_cache.sh` now looks for caches from commits earlier in the history. This means that currently, we can observe the following wasteful sequence of events in several PRs (e.g. leanprover-community#2258):

1. commit A gets pushed to a certain branch and CI_A starts.
2. While CI_A is still running the `leanpkg build` step, commit B is pushed and CI_B starts.
3. CI_A finishes `leanpkg build` but doesn't upload its cache to Azure because of leanprover-community#2048 
4. Now commit C is pushed and can't use the results of CI_A
5. CI_B finishes `leanpkg build` but doesn't upload its cache
6. Commit D is pushed and can't use the results of CI_A or CI_B ...

(This can keep happening as long as the next commit arrives before the most recent CI uploads to Azure).

I also cleaned up some stuff that was left over from when we ran the build on both "pr" and "push" events.
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
PR leanprover-community#2048 changed the CI so that olean caches are not pushed to Azure if a later commit on the same branch occurs. The reasoning was that it was unlikely that anyone would fetch those caches. That's changed as of leanprover-community#2278, since `fetch_olean_cache.sh` now looks for caches from commits earlier in the history. This means that currently, we can observe the following wasteful sequence of events in several PRs (e.g. leanprover-community#2258):

1. commit A gets pushed to a certain branch and CI_A starts.
2. While CI_A is still running the `leanpkg build` step, commit B is pushed and CI_B starts.
3. CI_A finishes `leanpkg build` but doesn't upload its cache to Azure because of leanprover-community#2048 
4. Now commit C is pushed and can't use the results of CI_A
5. CI_B finishes `leanpkg build` but doesn't upload its cache
6. Commit D is pushed and can't use the results of CI_A or CI_B ...

(This can keep happening as long as the next commit arrives before the most recent CI uploads to Azure).

I also cleaned up some stuff that was left over from when we ran the build on both "pr" and "push" events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants