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

Improve.ci+fixes #851

Closed
wants to merge 4 commits into from
Closed

Improve.ci+fixes #851

wants to merge 4 commits into from

Conversation

rivy
Copy link
Contributor

@rivy rivy commented Feb 17, 2019

I created an improved AppVeyor CI config (in the spirit of @haarg's Travis CI toolset) which I've been testing and using for a while and now I'm slowly releasing it into more public use. The supplied AppVeyor config (and related helper tools) is able to test all the way back to Perl v5.8.8.8 (the earliest Strawberry Perl available). Further information about the AppVeyor CI config is available at rivy/CI.AppVeyor.helpers-perl.

By expanding testing, I uncovered some bugs in the Module::Build requirements. I've included the fixes in three small annotated commits.

So, this PR compiles successfully all the way back to Perl v5.8.8.8 for MSWin32/AppVeyor and Perl v5.8 for Travis (see reports at AppVeyor CI and Travis CI). It also includes support for code coverage (see reports at CodeCov and Coveralls).

I hope this is acceptable, and let me know if you'd like any changes.

* fixes/improves AppVeyor CI

* similar in spirit to @haarg's Travis CI helper scripts

* *robustly flexible*, working with make, build, and prove-compatible distributions
* supports integration of both CodeCov and Coveralls coverage testing
* overcomes bugs in AppVeyor and perl tooling
* robust in cases of inaccessible Chocolatey.org or StrawberryPerl.com downloads

* ".appveyor.yml" can be a simple drop-in for most distributions

* see <https://github.com/rivy/CI.AppVeyor.helpers-perl> for more information
@petdance
Copy link
Member

Thanks. I'll try to take a look at it in the next few days.

Ultimately we'd like to get off of Module::Build entirely.

@rivy
Copy link
Contributor Author

rivy commented Feb 19, 2019

Thanks for the look.

Primarily, this is adding improved, more robust, AppVeyor testing with some added depth of version testing.

Doing that just uncovered the Module::Build requirement issues (which are quite small changes).

But this should actually help move off of Module::Build as the included newer AppVeyor config works fairly easily and automatically with build, make, or prove-compatible distributions.

@rivy
Copy link
Contributor Author

rivy commented Mar 29, 2019

@petdance , have you had a chance to look at this?

@petdance
Copy link
Member

@petdance , have you had a chance to look at this?

I'm sorry, I haven't. @oalders made some patches to the Travis config. Maybe he can check the AppVeyor config too?

@oalders
Copy link
Contributor

oalders commented May 14, 2019

I actually don't know my way around Appveyor, but maybe @genio would have an opinion?

@genio
Copy link

genio commented May 14, 2019

This is a bit more complex of an .appveyor.yml than I normally work with. Luckily, @haarg (I believe it was him anyway), wrote a great .appveyor.cmd to abstract out some of the complexity of setting up AppVeyor.

I took it a bit further in my UV distribution: .appveyor.cmd which allows me to write my .appveyor.yml a bit more simply.

Windows is, unfortunately, no fun to test on. Also, unfortunately, you won't really be able to test anything much on ActivePerl as it doesn't lend itself to build XS modules at all. @haarg's work does, however, allow you to test on CygWin fairly well. The overall complexity of testing on Windows is because there are many "environments" within the one environment:

  1. Windows (64-bit) + PowerShell (64-bit) + Strawberry (64-bit)
  2. Windows (64-bit) + PowerShell (64-bit) + Strawberry (32-bit)
  3. Windows (64-bit) + PowerShell (32-bit) + Strawberry (64-bit)
    1. and so on for the combinations of PowerShell and Windows 32v64
  4. Windows (64-bit) + Cmd.exe (64-bit)
    1. and so on for the combinations of Cmd and Windows 32v64
    2. yes, there has to be a distinction between PowerShell and Cmd. Some things work on one and not the other.
  5. Windows (64-bit) + MSys2
    1. Can we pretend this one doesn't exist?
  6. CygWin 64-bit
  7. CygWin 32-bit
  8. Windows + WSL (though this is tricky now that many flavors of linux can be used)
  9. Windows and the new conhost they're working on
  10. Windows and the new built-in linux kernel + WSL
  11. Windows and hand-built Perl (using MinGW or VisualStudio) or ActivePerl
    1. let's pretend these don't exist as well, please

Given all of that nonsense, I found it so complex that I stopped caring about anything other than Windows + Strawberry and Windows + Cygwin. While the others might be used, automating tests for them would be a nightmare and is nigh-impossible.

Also, worrying much about the 32v64-bit stuff also isn't necessary for most things (unless we're talking about DBD::ODBC or some other XS dists: you can't see 32-bit ODBC connections when running a 64-bit cmd/powershell and vice-versa.

The other issue with attempting to test on too many environments is that AppVeyor doesn't run tests in parallel and pulling down each version of Strawberry to test against already costs you a lot of time. I am, however, still looking into Microsoft's Azure Pipelines to possibly make use of containers for testing in the future.

Now that I've been long-winded enough for a year, take a look at the split-up AppVeyor config and see if it makes sense to you. If so, I'll be happy to take a stab at working something out for this dist that would behave similarly to what I've done using @haarg's work.

@petdance
Copy link
Member

petdance commented May 14, 2019

Testing the branch. Let's see what AppVeyor says.

@petdance
Copy link
Member

@petdance
Copy link
Member

Merged to dev on f98f779. Thank you.

@petdance petdance closed this May 15, 2019
@oalders
Copy link
Contributor

oalders commented May 15, 2019

Thanks for checking this over @genio!

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

Successfully merging this pull request may close these issues.

None yet

4 participants