- 
                Notifications
    You must be signed in to change notification settings 
- Fork 87
Move Linux CI jobs to GitHub Actions #1471
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you! A couple small comments.
        
          
                .bazelrc
              
                Outdated
          
        
      | # ------------------------------ | ||
| build:linux-nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host | ||
| build:macos-nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host | ||
| build:nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the config flags documented in the README. With this change the last reference to linux-nixpkgs is gone and --config=linux-nixpkgs becomes invalid. I'd prefer to keep the config matrix in the README valid for symmetry with the bindist cases and in case we need platform specific flags in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bazel doesn't appear to support empty configurations (would make a good feature request upstream). We could hack around that, but I opted instead to remove this configuration from the README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened such a feature request: bazelbuild/bazel#12844.
I don't think that change is correct. On Linux/Nixpkgs we would need --config=nixpkgs but the README claims (unneeded). On macOS we would now need both --config=nixpkgs --config=macos-nixpkgs but the README claims --config=macos-nixpkgs. The aim of #1301 was to require only one top-level --config flag for developers.
I've pushed a commit that loads --config=nixpkgs from both linux-nixpkgs and macos-nixpkgs. With this developers only need a single top-level config and both linux-nixpkgs and macos-nixpkgs unfold the nixpkgs config as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow. --config=nixpkgs doesn't exist in this branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, spurious comment. I had the wrong branch checked out.
        
          
                .buildkite/pipeline.yml
              
                Outdated
          
        
      | @@ -1,45 +0,0 @@ | |||
| steps: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/RunTests.hs now contains dangling references to .circleci and .buildkite in comments that need to be fixed.
bac7408    to
    47b83e0      
    Compare
  
    | @aherrmann just realized that yesterday when I did the force push, this somehow went to  This is probably why  I use Magit, which has a concept of "upstream" and "push" branch: https://magit.vc/manual/magit/The-Two-Remotes.html. This is likely culprit. I noticed before that it interacts badly with using Git CLI. | 
| 
 Hmm, I thought branch protection on  | 
| AFAIU branch protection only protects against force pushes from admins.
Though we can enable stronger protection. | 
| 
 I see, if I understand these docs correctly then admins can always push. 
 | 
| This was failing on buildifier warnings | 
| That looks like some potential network causing a timeout to be hit - the invocation uploaded just fine on our end, and I can't find any errors in the logs related to that invocation: Are you seeing this consistently? | 
| Thanks for looking into this @siggisim . 
 No, this was the first time I noticed this. Yes, looks like network flakiness. I'll let you know if this happens repeatedly. | 
This reverts commit 6a7d835.
It now points you to the master branch history.
f5249b6    to
    3be1b15      
    Compare
  
    Tested on #1471 in Mergify Config Editor UI.
| I've included an update to the mergify configuration and checked it in their config editor UI. | 
Hot on the heals of #1460, we move the Linux jobs to GitHub Actions as
well. In practice, BuildKite with custom runners wasn't buying us that
much time, though we may return to custom runners (via Actions) later.
The only CI job not on Actions is now the Windows one.