-
-
Notifications
You must be signed in to change notification settings - Fork 339
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: Switch Travis to Ubuntu 22.04 and fix non-prototype in ODBC #3002
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflicts must be solved.
I verified that the file used that lists the dependencies included the change that was conflicting, it removed python-six and python-six was also removed from that file. |
Conflicts were resolved manually, so review was addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I investigated the Travis failure and include a fix, but needs to be committed as I can't add an suggestion on lines not changed in the PR.
@wenzeslaus I made a PR to your branch with the fixes that I found were needed but couldn't apply a suggestion on the lines. |
Pinging back, so you could take a look at the PR in your fork targetting this branch, at the time that I made the changes it would have fixed the blockers in the PR and would make it ready to merge. |
Thanks @echoix. The Bash helped. If you want to take over here. Feel free to edit the branch directly. In any case, the current state is that it fails with PDAL (I didn't investigate):
|
Can you make sure that the box allow edits by maintainers is checked? I think I tried and it didn't work at the time |
Both are on "Maintainers are allowed to edit this pull request." and "Allow edits and access to secrets by maintainers" |
I'm investigating on lost time here and then when I get an idea. What I realized is that on Github CI, we use pdal 2.3.0 from ubuntu jammy. We never really built with pdal 2.6.2. Are we sure that our conifgure script works with pdal 2.6.3, or anything past 2.3.0? If someone could answer that, it might save me a little investigation instead of trying to get that answer by myself. |
I have tried your changes in a According to https://repology.org/project/pdal/versions a few distros already include 2.6.3 but I didn't try that. |
It's (one of) the only place where we are building with ubuntugis unstable. Ubuntugis unstable gives 2.6.3. It had a new build between Thursday and this weekend, since a problem with a plugin package caused QGIS problems. But it shouldn't be that related, it was CMake stuff if I understand correctly. I don't know the purpose of the Travis builds, so at least keeping it different of the GitHub Actions builds we already is important I think. Ubuntu 22.04 on GitHub gives us 2.3.0 from Ubuntu's default repos. |
The Travis build was there before GitHub Actions were available. Then there was some time when GHA were broken and Travis provided the only (backup?) CI check (visual and doable as there were no required checks). Now perhaps, the value is in having another CI in place, so if our primary one goes away, we have a backup ready to go. I think I have seen even with Travis, but definitively in other cases that differences in configuration or tooling (and some duplication) make the CI more robust overall (e.g., one run is okay but another fails because of just slightly older dependency).
That seems to be a valid approach. ...if you think it is not too much trouble for the one or two above values it provides. |
Ok, it seems like an incompatibility of some sort with a newer PDAL or with the ubuntugis PPA (that provides a newer PDAL version). Disabling the ubuntugis ppa allows the ./configure call to succeed. It shouldn’t be an issue limited to Travis. It should be possible to reproduce somewhere else. Our builds not being successful with the Ubuntugis ppa is somewhat problematic in my opinion |
Is the error anywhere visible? PDAL 2.7.0 just hit Fedora (https://src.fedoraproject.org/rpms/PDAL/c/3f731663ed3cbd4fbbacf475c1fb9a1bb0492b24?branch=rawhide), apparently without problems. |
@echoix Can you please take over? |
Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point. Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can only comment as a co-author, but I approve. I don't know why the functions need to be fixed only now, but I assume we compile without ODBC in GitHub Actions CI.
@landam Please, check this. |
The -Wdeprecated-non-prototype errors are now fixed and clang and gcc pass Travis builds using -Werror. Ready to merge. Does the title need to change to say some C files were changed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved as much as I can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed with Vaclav, as he can't approve a PR opened by himself, I'm setting the approval
…o#3002) * CI: Switch Travis to Ubuntu 22.04 (jammmy) Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point. Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation. * Update wxgtk pkg to 4.0 * Replace custom install list by apt.txt file from GitHub Actions * Change shebang to use bash instead of POSIX sh (#3) * CI(travis): Add --with-pdal in linux.script.sh * CI(travis): Remove sudo: required as it has no effect * CI(travis): Fix warning os and dist missing from root * CI(travis): Fix warning matrix is an alias for jobs * CI(travis): Run make with make -j $(nproc) * CI(travis): Show MAKEFLAGS at start of script * CI(travis): use c++17 standard * CI(travis): Remove duplicated dist key in include array * CI(travis): Remove invalid --with-python from configure flags * CI(travis): Remove unused codecov upload * CI(travis): Remove irc notification * CI(travis): Add -Werror and -fPIC to CFLAGS and CXXFLAGS on make call * CI(travis): Limit runs on branches to main and release branches * CI(travis): Add -Wfatal-errors * db: Fix -Wdeprecated-non-prototype in describe.c for clang builds * CI(travis): Add --no-keep-going in MAKEFLAGS to stop on errors * db: Fix -Wdeprecated-non-prototype in odbc driver's fetch.c for clang builds * db: Fix -Wdeprecated-non-prototype in odbc driver's listtab.c for clang builds --------- Co-authored-by: Edouard Choinière <[email protected]>
…o#3002) * CI: Switch Travis to Ubuntu 22.04 (jammmy) Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point. Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation. * Update wxgtk pkg to 4.0 * Replace custom install list by apt.txt file from GitHub Actions * Change shebang to use bash instead of POSIX sh (OSGeo#3) * CI(travis): Add --with-pdal in linux.script.sh * CI(travis): Remove sudo: required as it has no effect * CI(travis): Fix warning os and dist missing from root * CI(travis): Fix warning matrix is an alias for jobs * CI(travis): Run make with make -j $(nproc) * CI(travis): Show MAKEFLAGS at start of script * CI(travis): use c++17 standard * CI(travis): Remove duplicated dist key in include array * CI(travis): Remove invalid --with-python from configure flags * CI(travis): Remove unused codecov upload * CI(travis): Remove irc notification * CI(travis): Add -Werror and -fPIC to CFLAGS and CXXFLAGS on make call * CI(travis): Limit runs on branches to main and release branches * CI(travis): Add -Wfatal-errors * db: Fix -Wdeprecated-non-prototype in describe.c for clang builds * CI(travis): Add --no-keep-going in MAKEFLAGS to stop on errors * db: Fix -Wdeprecated-non-prototype in odbc driver's fetch.c for clang builds * db: Fix -Wdeprecated-non-prototype in odbc driver's listtab.c for clang builds --------- Co-authored-by: Edouard Choinière <[email protected]>
Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point.
Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation.