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

ci: cleanup MSYS2 related builds and scripts #11124

Merged
merged 7 commits into from
Jan 28, 2023

Conversation

kasper93
Copy link
Contributor

Frankly I don't care about msys2 build itself, but isn't it annoying for you guys that ci is red on master branch? This fixes timeout of appveyor and add github workflow, because why not.

@kasper93 kasper93 force-pushed the msys2_clean branch 2 times, most recently from e4c3f68 to 7241b03 Compare January 11, 2023 00:46
@Dudemanguy
Copy link
Member

Truthfully, I did not even notice appveyor was failing until earlier this week... the time spent fixing it is appreciated, but I'm not even sure if we still want it. Does it really do us any good? Maybe we should just keep the MSYS2 to github actions move and delete appveyor (there were previous PR attempts at this actually). Not sure what everyone else thinks.

@kasper93
Copy link
Contributor Author

kasper93 commented Jan 11, 2023

I will let you decide. Indeed having msys2 build both in GH Actions and AppVeyor is kind of the same, but the overhead of having both is minimal, hence I didn't propose in this PR to remove the AppVeyor one. If anything this would allow to simplify it even further, but not much difference really.

EDIT: Without appveyor it would look like this kasper93@1957ca5 could also add waf, but it fails in my test VM and I didn't look why.

@sfan5
Copy link
Member

sfan5 commented Jan 13, 2023

Personally I'm fine with just fixing appveyor and continuing to use it.

TOOLS/appveyor-build.sh Outdated Show resolved Hide resolved
@Biswa96
Copy link
Contributor

Biswa96 commented Jan 16, 2023

msys2 does not use WAF build system. Removing it from CI would be another choice. If you want to keep old slower Appveryor adding export WAF_NO_PREFORK=1 command before waf configure would help. Like this

--- a/TOOLS/appveyor-build.sh
+++ b/TOOLS/appveyor-build.sh
@@ -6,6 +6,7 @@ export CC=gcc
 export PKG_CONFIG=/usr/bin/pkg-config
 export PERL=/usr/bin/perl
 export PYTHON=/usr/bin/python3
+export WAF_NO_PREFORK=1
 
 "$PYTHON" bootstrap.py
 "$PYTHON" waf configure \

@kasper93
Copy link
Contributor Author

Yes, I know. There is also another underlying issue with waf on MSYS2. I didn't have time to push changes, but it was working last week, just need to put in on this branch.

@kasper93
Copy link
Contributor Author

kasper93 commented Jan 17, 2023

Ok, updated. After looking at this I decided to remove AppVeyor. It does not bring any value, it is run only on master branch (and no one is looking at the status) and is exactly the same environment that I added just now to GH actions, which will run also on PRs. I'm open to discussion, having it supported is just a few scripts, but it really would be the same as current gh build...

To summary the changes:

  • Remove AppVeyor
  • Add MSYS2 meson and waf build to GH Actions
  • Fixed egl-angle-lib option in meson
  • Removed version.sh used by waf and migrated to version.py (this actually both fixes MSYS2 build and well removes duplication)
  • Minor readme update

overall small changes...

@kasper93 kasper93 force-pushed the msys2_clean branch 2 times, most recently from 76592b6 to c45eda2 Compare January 17, 2023 21:32
@Dudemanguy
Copy link
Member

I see that you put the commands for the msys2 build directly in the github workflow. Could you instead rework it so it's in its own build-msys2.sh like the other workflows?

@kasper93
Copy link
Contributor Author

@Dudemanguy done

ci/build-msys2.sh Show resolved Hide resolved
version.py Outdated Show resolved Hide resolved
video/out/opengl/angle_dynamic.c Show resolved Hide resolved
Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't verify the waf weirdness or the zimg test thing myself, but I'll assume they work.

@kasper93 kasper93 force-pushed the msys2_clean branch 3 times, most recently from 6997b1f to adcfc86 Compare January 21, 2023 12:48
wscript Show resolved Hide resolved
@Dudemanguy Dudemanguy merged commit 9d659ed into mpv-player:master Jan 28, 2023
@kasper93 kasper93 deleted the msys2_clean branch March 1, 2023 03:00
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.

4 participants