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

add workflow doing windows builds with qt5/6 and osgeo4w dependencies #57445

Closed
wants to merge 4 commits into from

Conversation

jef-n
Copy link
Member

@jef-n jef-n commented May 15, 2024

  • remove unused osgeo4w and NSIS packaging files and disabled azure-pipeline build
  • integrate osgeo4w patches currently used for qgis-qt6-dev (superceeds changes to support msvc builds with qt6 #56980)
  • also adds pr comments to artifacts and dashboard test results

@github-actions github-actions bot added this to the 3.38.0 milestone May 15, 2024
- name: 'Post artifact download link and test results as comment on PR'
uses: actions/github-script@v7
if: github.event_name == 'pull_request'
with:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jef-n I don't think this will work for pull requests made from forks -- the workflow won't have the permission to create comments on the qgis repo. That's why I had to go via the two workflow approach for the ming artifact comments...

@nyalldawson
Copy link
Collaborator

@jef-n @m-kuhn @MehdiChinoune

If we're adding two windows builds here + 1 from #57414, can we at least drop the existing mingw64 and msys2 workflows? I think 5 separate windows builds is tending slightly toward overkill 🤣

Copy link

github-actions bot commented May 16, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit beea934)

@m-kuhn
Copy link
Member

m-kuhn commented May 16, 2024

@jef-n @m-kuhn @MehdiChinoune

If we're adding two windows builds here + 1 from #57414, can we at least drop the existing mingw64 and msys2 workflows? I think 5 separate windows builds is tending slightly toward overkill 🤣

😆 I agree

I also think the user communication should be quite clear in what should be tested. I.e. the build that is becoming the stable artifact (at the moment qt5) should be most prominent and everything else can be in small print/collapsed etc. Ideally everything in a single comment to not clutter the communication in the PR's with too many comments.

@troopa81
Copy link
Contributor

If we're adding two windows builds here + 1 from #57414, can we at least drop the existing mingw64 and msys2 workflows? I think 5 separate windows builds is tending slightly toward overkill 🤣

I agree. And do we really need even 3, it's gonna overload the CI pipeline for basically building the same code with the same compiler. Can we not have qt5 with osgeo4w and vcpkg for qt6 ?

@3nids
Copy link
Member

3nids commented May 28, 2024

I agree. And do we really need even 3, it's gonna overload the CI pipeline for basically building the same code with the same compiler. Can we not have qt5 with osgeo4w and vcpkg for qt6 ?

+1
One thing is that we are always fighting with the cache. We need to lower the number of workflows using cache as much as possible.

@jef-n
Copy link
Member Author

jef-n commented May 28, 2024

why do we need vcpkg?

@troopa81
Copy link
Contributor

why do we need vcpkg?

I am not expert with vcpkg but IMHO I see a lot of advantages:

  • It's native on Windows and supported by Microsoft
  • There are a lot of already packaged libraries which avoid the burden to have to package everything ourselves
  • It works well with CMake
  • All the build instructions can live in QGIS repository as showed in Windows Qt6 builds based on vcpkg  #57414, easing the contribution process and consistency with source code

There is a more thorough explanation on why vcpkg on the related QEP.

@nyalldawson
Copy link
Collaborator

Just a heads up -- we'll need to pin Qt at 6.6 for now, due to broken 3d in 6.7 (see #57640) and regressions in svg rendering

Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added stale Uh oh! Seems this work is abandoned, and the PR is about to close. and removed stale Uh oh! Seems this work is abandoned, and the PR is about to close. labels Jun 25, 2024
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 10, 2024
@jef-n jef-n closed this Jul 15, 2024
@jef-n jef-n reopened this Jul 15, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 15, 2024
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 30, 2024
Copy link

github-actions bot commented Aug 6, 2024

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

@github-actions github-actions bot closed this Aug 6, 2024
@jef-n jef-n reopened this Aug 6, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 6, 2024
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 21, 2024
Copy link

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Uh oh! Seems this work is abandoned, and the PR is about to close.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants