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.

Sign up for GitHub

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

Jump to bottom

CI: introduce setup-msys2@v2 setup action #2508

Merged
merged 8 commits into from
Aug 4, 2022
Merged

CI: introduce setup-msys2@v2 setup action #2508

merged 8 commits into from
Aug 4, 2022

Conversation

ninsbl
Copy link
Member

@ninsbl ninsbl commented Aug 2, 2022

For using MSYS in CI (esp. github) it is recommended to use msys2/setup-msys2@v2 action. See also: https://www.msys2.org/docs/ci/

This PR introduces that action and should make the build pass again without errors.

Replaces #2468

Should also speedup the test a few minutes and maybe simplifies to explore other possibilities (e.g. using UCRT)

@ninsbl ninsbl added backport_needed windows Microsoft Windows specific CI Continuous integration labels Aug 2, 2022
@ninsbl ninsbl added this to the 8.2.1 milestone Aug 2, 2022
@ninsbl ninsbl requested review from neteler and hellik August 2, 2022 13:08
@ninsbl ninsbl mentioned this pull request Aug 2, 2022
Closed
@ninsbl
Copy link
Member Author

ninsbl commented Aug 2, 2022

Hmmm.... This seem to resurface #1697 as compilation now fails with:

D:\a\grass\grass\lib\cairodriver/text.c:207: undefined reference to `cairo_ft_font_face_create_for_pattern'
collect2.exe: error: ld returned 1 exit status

@nilason any idea?

hellik
hellik reviewed Aug 2, 2022
View reviewed changes
Copy link
Member

@hellik hellik left a comment

Choose a reason for hiding this comment

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

The yml changes are looking good.

Regarding the cairo issue, some more checks why it pops up again may be good.

@ninsbl
Copy link
Member Author

ninsbl commented Aug 3, 2022

Working now...

Since OSGeo4W checks succeed for #2506 too, it seems the issue was a bug in MSYS2. So, I guess checks would now pass also without these changes. Seems to be messed up OSGeo4W vs. MSYS2 cairo...

Still, the PR makes the workflow a bit cleaner and avoids mixing OSGeo4W and MSYS2 cairo library. And using msys2/setup-msys2@v2 seems to be an advantage anyway, so I thing the changes are still an improvement...

BTW: cairo-devel from OSGeo4W does not seem to provide a cairo.pc, thus the warning about pkg-config...

@ninsbl ninsbl requested a review from hellik August 3, 2022 06:52
neteler
neteler approved these changes Aug 3, 2022
View reviewed changes
Copy link
Member

@neteler neteler left a comment

Choose a reason for hiding this comment

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

Great work - I cannot judge in detail but the fact that the CI job finally compiles again is a good sign :)

hellik
hellik approved these changes Aug 3, 2022
View reviewed changes
Copy link
Member

@hellik hellik left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks the PR!

@nilason
Copy link
Contributor

nilason commented Aug 3, 2022

Hmmm.... This seem to resurface #1697 as compilation now fails with:

D:\a\grass\grass\lib\cairodriver/text.c:207: undefined reference to `cairo_ft_font_face_create_for_pattern'
collect2.exe: error: ld returned 1 exit status

@nilason any idea?

I did take a look at the Cairo code and couldn't find any related changes there. I suppose(d) it was build set up related somehow... Great you found the way!

@ninsbl ninsbl merged commit 53b69fb into OSGeo:main Aug 4, 2022
ninsbl added a commit to ninsbl/grass that referenced this pull request Aug 4, 2022
@ninsbl
backport OSGeo#2508
25d5677
ninsbl added a commit to ninsbl/grass that referenced this pull request Aug 4, 2022
@ninsbl
backport OSGeo#2508
dac9d2e
@ninsbl ninsbl mentioned this pull request Aug 4, 2022
Merged
ninsbl added a commit that referenced this pull request Aug 4, 2022
@ninsbl
CI: introduce setup-msys2@v2 setup action - backport #2508 (#2510)
c777736 
* backport #2508
@ninsbl ninsbl mentioned this pull request Aug 4, 2022
Closed
@ninsbl
Copy link
Member Author

ninsbl commented Aug 4, 2022

Backported in #2510

@wenzeslaus
Copy link
Member

FYI, the OSGeo4W check passed four days ago on August 1 for c0a83a3. That's before this was merged in 53b69fb. See the recent commits on main:

https://github.com/OSGeo/grass/commits/main

ninsbl added a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@ninsbl
CI: introduce setup-msys2@v2 setup action (OSGeo#2508)
4cf6021 
Thanks @nilason, @neteler and @hellik for your feedback. It has indeed been a build env issue.
Will create a follow up PR for backporting...

* use msys2-setup

* specify location

* get OSGeo4W includes for cairo

* try to deactivate fontconfig

* remove msys2 cairo

* link osgeo4w cairo

* link osgeo4w cairo libs

* no fontconfig
ninsbl added a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@ninsbl
CI: introduce setup-msys2@v2 setup action (OSGeo#2508)
f48211c 
Thanks @nilason, @neteler and @hellik for your feedback. It has indeed been a build env issue.
Will create a follow up PR for backporting...

* use msys2-setup

* specify location

* get OSGeo4W includes for cairo

* try to deactivate fontconfig

* remove msys2 cairo

* link osgeo4w cairo

* link osgeo4w cairo libs

* no fontconfig
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@ninsbl @neteler
CI: introduce setup-msys2@v2 setup action (OSGeo#2508)
c503966 
Thanks @nilason, @neteler and @hellik for your feedback. It has indeed been a build env issue.
Will create a follow up PR for backporting...

* use msys2-setup

* specify location

* get OSGeo4W includes for cairo

* try to deactivate fontconfig

* remove msys2 cairo

* link osgeo4w cairo

* link osgeo4w cairo libs

* no fontconfig
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neteler neteler neteler approved these changes

@hellik hellik hellik approved these changes

Assignees
No one assigned
Labels
CI Continuous integration windows Microsoft Windows specific
Projects
None yet
Milestone
8.2.1
Development

Successfully merging this pull request may close these issues.
5 participants
@ninsbl @nilason @wenzeslaus @neteler @hellik