Added GitHub Actions workflows #506

nmoinvaz · 2020-06-06T16:27:05Z

Description:

On each commit these workflows will be run to verify that project generation, source file compilation, and test cases run successfully.
On each pull request a workflow will be run to verify that all fuzzing tests pass successfully.

Supports the following CI configurations:

CMake
- Ubuntu GCC
- Ubuntu GCC OSB (out of source build)
- Ubuntu Clang
- Ubuntu Clang Debug
- Windows MSVC Win32
- Windows MSVC Win64
- macOS Clang
- macOS GCC
Configure
- Ubuntu GCC
- Ubuntu GCC OSB (out of source build)
- macOS GCC
Fuzz
- OSS-Fuzz on pull request only

Example runs can be found here:
https://github.com/nmoinvaz/zlib/actions

This PR does not change any source code and only adds yaml workflows. Take from this PR what you need.

nmoinvaz · 2022-03-29T15:42:56Z

@madler consider getting rid of appveyor and use GitHub Actions.

…est and because of madler#506

Neustradamus · 2022-08-18T21:42:29Z

@madler: What do you think?

nmoinvaz · 2022-08-18T22:02:27Z

I did some minor cleanup incase anybody cares.

ProgramMax · 2022-08-18T22:05:09Z

FWIW, I support this. :)
The changes from https://github.com/madler/zlib/pull/492/files test/example.c would still be helpful.

Back when that other pull request was proposed, there was concern within Google about Github Actions not being secure enough. That has since changed. And given the change, I recommend using Github Actions instead of AppVeyor.

It will be much easier for everyone involved, too. You'll recall last time we tried to get AppVeyor running it had more steps (each of which required confirming it is safe) than it was worth. With this, we won't need those annoying steps.

It might be good to land this first. Then I can update the other pull request to not include any AppVeyor parts. That'll bring the useful test/example.c changes in.

madler · 2022-08-23T20:25:33Z

So I just add these files, and then, magic? Are there any settings or permissions on GitHub needed?

I think I have turned off appveyor.

nmoinvaz · 2022-08-23T20:33:56Z

Correct, magic, if you build it they will come.

nmoinvaz · 2022-08-23T20:40:53Z

By the way, something I did not add was the ability to check to see that contribs compiled. Perhaps contrib authors can add those workflows. And I did not add checks for the vcproj files you have. Only checked cmake and configure.

If there is interest I can look into some of that.

madler · 2022-08-23T21:40:06Z

Pull requests need to be on develop, not master. @nmoinvaz Can you resubmit?

nmoinvaz · 2022-08-23T21:41:57Z

They should be activated for all branches. Are you saying you only want for develop?

madler · 2022-08-23T21:51:00Z

Yes, develop. master is only for releases. When there is a new release, the files will end up on master as well.

nmoinvaz · 2022-08-23T21:59:06Z

You should be able to press the Edit button on this PR on the top right and change it from master to develop.

madler · 2022-08-23T22:04:37Z

Didn't know I could do that. Thanks!

ProgramMax · 2022-08-23T22:05:58Z

I'll submit a pull request to get rid of AppVeyor. That "build failed" just then is annoying. :)

ProgramMax · 2022-08-23T22:11:55Z

Oh. The AppVeyor files didn't land. I think the error is because the AppVeyor account is linked here but isn't finding any files instructing it.

You can probably log into AppVeyor and remove zlib from the repos for it to check.

madler · 2022-08-23T22:16:11Z

@ProgramMax I have deleted zlib from appveyor. Hopefully it has been exorcised now.

ProgramMax · 2022-08-23T22:17:50Z

Sounds good.
It is likely still showing up here because there have been no code changes and GitHub remembers the last CI results. But future changes should be fine.

.github/workflows/cmake.yml

.github/workflows/configure.yml

.github/workflows/fuzz.yml

.github/workflows/cmake.yml

.github/workflows/configure.yml

Croydon · 2022-08-24T23:10:18Z

Please don't force push into pull requests when it isn't necessary for the future. Reviewers have to start again to review the entire pull request instead of just the new changes. Additionally, you don't give any credits to the reviewers for code suggestions.

nmoinvaz · 2022-08-24T23:37:01Z

Please don't force push into pull requests when it isn't necessary for the future. Reviewers have to start again to review the entire pull request instead of just the new changes.

There are pros and cons of each way.

Additionally, you don't give any credits to the reviewers for code suggestions.

I'm not sure that it makes sense for EVERY suggestion to be credited. The commits in zlib are always authored by @madler and he can credit whoever he wants - even all 6 participants on this PR. After having it open for 2 years I would be happy if the changes are merged even if I don't get credit. A lot of the changes I made since last push were already made by me in zlib-ng GHA workflows (which is where these workflows come from) but this PR was never updated, your suggestions I have already made there even, although the GCC issue was only partially made in zlib-ng already.

.github/workflows/cmake.yml

ProgramMax · 2022-09-26T21:34:11Z

.github/workflows/cmake.yml

+          - name: Ubuntu GCC
+            os: ubuntu-latest
+            compiler: gcc
+
+          - name: Ubuntu GCC OSB
+            os: ubuntu-latest
+            compiler: gcc
+            build-dir: ../build
+            build-src-dir: ../zlib


What is "OSB" here?

Also, isn't it normal when using CMake to:
$ mkdir build
$ cd build
$ cmake ..
Should all the CMake builds be doing that? (Which is similar but not the same to what OSB is doing.)

OSB means out of source build, meaning build in a separate directory from the source directory, which is the typical case. I have reversed it, so there is now a "ISB" aka in-source-build run.

I now realize why I did it in reverse, because GitHub does not allow you to upload a configure.log from outside the source directory.

ProgramMax · 2022-09-26T21:37:39Z

.github/workflows/fuzz.yml

+      uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
+      with:
+        oss-fuzz-project-name: 'zlib'
+        fuzz-seconds: 600


I'm curious about a 10 minute fuzzer run.

On one hand, I like the idea of making sure the fuzzer works. That way we don't accidentally break it. That is the benefit of having it in the CI.

But on the other hand, we are unlikely to find problems in only 10 minutes. And we don't need it to run for 10 minutes to make sure it wasn't broken. Also, I don't think anyone wants to wait around for 10 minutes before learning if the CI approves their change.

Could this just be 30s? Do we actually want 10 minutes of fuzzing here?

10 minutes might be long as you say. But there is also a cost to spooling up a new CI instance for GH so it should probably be greater than 30 seconds. I think 5 minutes is a good medium.

nmoinvaz · 2022-10-06T21:47:33Z

Rebased.

On each commit these workflows will be run to verify that project generation, source file compilation, and test cases run successfully. On each pull request a workflow will be run to verify that all fuzzing tests pass successfully. madler#492

madler · 2022-10-07T04:58:58Z

Incorporated.

madler · 2022-10-10T19:51:02Z

Thank you so much @nmoinvaz for this! It seems to working great.

I say "seems", only because it's always giving a green check mark. I find this suspicious. I went in and looked at the output, and I didn't even see any warnings. Will this show or flag warnings from the compilations? Or only errors?

ProgramMax · 2022-10-10T20:22:08Z

Agreed. Thank you for doing this, @nmoinvaz.

Warnings are allowed by default. Errors will not give a green check mark. We can add a flag to treat warnings as errors if we wanted.
Any step that has a non-zero exit code (IE returning -1 from main()) will prevent the green checkmark.

It does seem strangely quiet, though. Things which I thought would return non-zero aren't.

There are 3 jobs: Ubuntu GCC, Ubuntu GCC ISB, and macOS GCC.
Only the Ubuntu GCC ISB is really working. The other two print "Please use ./configure first. Thank you." and "make: Nothing to be done for 'test'.".

So I think we have some follow-up work to do. But it is correctly building and testing on Ubuntu GCC ISB.

ProgramMax · 2022-10-10T20:26:09Z

Oops. Let me correct myself:
There are 3 workflows. The "Configure" workflow behaves the way I described above.
There is also a "CMake" workflow that builds and tests correctly on all of its platforms.
Then there is an "OSS-Fuzz" workflow which hasn't run yet.

madler · 2022-10-10T20:27:16Z

What is "GCC ISB"?

ProgramMax · 2022-10-10T20:35:05Z

ISB = In-Source Build.

For CMake, it is common to:
$ mkdir build
$ cd build
$ cmake ..
Then all build files are inside the build/ subdirectory, rather than scattered within the code.
The non-ISB jobs do this. The ISB job does not.

nmoinvaz · 2022-10-10T22:10:48Z

@ProgramMax is right.

And configure working-directory steps should be changed to:

When I created this PR 2 years ago, there were some CI instances I knew weren't working, but I decided to only go with those that were working first (I don't remember which ones failed). It also may have been something incompatible with zlib CMake/configure itself.

I can add more CI instances to the workflows (ARM, etc) in another PR.

nmoinvaz · 2022-10-10T22:31:09Z

Also we don't check building of any contribs.

madler · 2022-10-11T00:29:14Z

Also we don't check building of any contribs.

That's fine for now. The only one we might eventually want to add is minizip. I check that one in my internal tests.

nmoinvaz mentioned this pull request Jun 6, 2020

Add continuous integration (CI) #492

Closed

nmoinvaz mentioned this pull request Apr 9, 2022

Corrected date from 2017 to 2022 #632

Closed

0ric1 added a commit to 0ric1/zlib that referenced this pull request Apr 9, 2022

Remove: cmake.yml, because it does not belong to the actual pull requ…

6e3090f

…est and because of madler#506

nmoinvaz force-pushed the improvements/ci-cmake branch from 138b8f4 to 79a98fb Compare August 18, 2022 22:01

Neustradamus mentioned this pull request Aug 18, 2022

Please make new release #686

Closed

madler changed the base branch from master to develop August 23, 2022 22:04

Croydon reviewed Aug 24, 2022

View reviewed changes

.github/workflows/cmake.yml Outdated Show resolved Hide resolved

Croydon reviewed Aug 24, 2022

View reviewed changes

.github/workflows/configure.yml Outdated Show resolved Hide resolved

Croydon reviewed Aug 24, 2022

View reviewed changes

.github/workflows/fuzz.yml Outdated Show resolved Hide resolved

giordano reviewed Aug 24, 2022

View reviewed changes

.github/workflows/cmake.yml Outdated Show resolved Hide resolved

giordano reviewed Aug 24, 2022

View reviewed changes

.github/workflows/configure.yml Outdated Show resolved Hide resolved

nmoinvaz force-pushed the improvements/ci-cmake branch 3 times, most recently from 0c61909 to 75fc8b3 Compare August 24, 2022 20:15

ProgramMax reviewed Sep 26, 2022

View reviewed changes

nmoinvaz force-pushed the improvements/ci-cmake branch 6 times, most recently from 00e55c2 to 76ecc86 Compare October 6, 2022 21:46

nmoinvaz force-pushed the improvements/ci-cmake branch from 76ecc86 to 5236324 Compare October 6, 2022 21:51

madler closed this Oct 7, 2022

nmoinvaz mentioned this pull request Oct 11, 2022

Additional improvements to GitHub Workflows #722

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added GitHub Actions workflows #506

Added GitHub Actions workflows #506

nmoinvaz commented Jun 6, 2020 •

edited

Loading

nmoinvaz commented Mar 29, 2022

Neustradamus commented Aug 18, 2022

nmoinvaz commented Aug 18, 2022

ProgramMax commented Aug 18, 2022

madler commented Aug 23, 2022 •

edited

Loading

nmoinvaz commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

madler commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

Croydon commented Aug 24, 2022

nmoinvaz commented Aug 24, 2022 •

edited

Loading

ProgramMax Sep 26, 2022

nmoinvaz Oct 6, 2022

nmoinvaz Oct 11, 2022

ProgramMax Sep 26, 2022

nmoinvaz Oct 6, 2022

nmoinvaz commented Oct 6, 2022

madler commented Oct 7, 2022

madler commented Oct 10, 2022 •

edited

Loading

ProgramMax commented Oct 10, 2022

ProgramMax commented Oct 10, 2022

madler commented Oct 10, 2022

ProgramMax commented Oct 10, 2022

nmoinvaz commented Oct 10, 2022 •

edited

Loading

nmoinvaz commented Oct 10, 2022

madler commented Oct 11, 2022

Added GitHub Actions workflows #506

Added GitHub Actions workflows #506

Conversation

nmoinvaz commented Jun 6, 2020 • edited Loading

nmoinvaz commented Mar 29, 2022

Neustradamus commented Aug 18, 2022

nmoinvaz commented Aug 18, 2022

ProgramMax commented Aug 18, 2022

madler commented Aug 23, 2022 • edited Loading

nmoinvaz commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

nmoinvaz commented Aug 23, 2022

madler commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

madler commented Aug 23, 2022

ProgramMax commented Aug 23, 2022

Croydon commented Aug 24, 2022

nmoinvaz commented Aug 24, 2022 • edited Loading

ProgramMax Sep 26, 2022

Choose a reason for hiding this comment

nmoinvaz Oct 6, 2022

Choose a reason for hiding this comment

nmoinvaz Oct 11, 2022

Choose a reason for hiding this comment

ProgramMax Sep 26, 2022

Choose a reason for hiding this comment

nmoinvaz Oct 6, 2022

Choose a reason for hiding this comment

nmoinvaz commented Oct 6, 2022

madler commented Oct 7, 2022

madler commented Oct 10, 2022 • edited Loading

ProgramMax commented Oct 10, 2022

ProgramMax commented Oct 10, 2022

madler commented Oct 10, 2022

ProgramMax commented Oct 10, 2022

nmoinvaz commented Oct 10, 2022 • edited Loading

nmoinvaz commented Oct 10, 2022

madler commented Oct 11, 2022

nmoinvaz commented Jun 6, 2020 •

edited

Loading

madler commented Aug 23, 2022 •

edited

Loading

nmoinvaz commented Aug 24, 2022 •

edited

Loading

madler commented Oct 10, 2022 •

edited

Loading

nmoinvaz commented Oct 10, 2022 •

edited

Loading