GHA: use Ubuntu 22.04 - #943
Conversation
KrisThielemans
left a comment
There was a problem hiding this comment.
I guess you want to split #937 into 2. However, you currently seem to default to not using the Ubuntu system libraries, but
- it will surely break docker for the moment (but I suppose you'll address that later)
- it will break the VM.
- wiki instructions https://github.com/SyneRBI/SIRF/wiki/SIRF-SuperBuild-Ubuntu#1-install-dependencies-via-apt will need updating as they point to an outdated version (this is probably a bad idea anyway)
Also, come to think of it, why "UBUNTU" in USE_UBUNTU_SYSTEM_LIBRARIES? (It'll work on any debian system).
In any case, do you really want the USE_UBUNTU_SYSTEM_LIBRARIES stuff in this PR?
| golang | ||
| pkg-config | ||
|
|
||
| if [ ${USE_UBUNTU_SYSTEM_LIBRARIES} ] ; then |
There was a problem hiding this comment.
this will fail if it isn't set, and in any case, test 1 is probably a very strange thing. I think you need something like
# set default value
: ${USE_UBUNTU_SYSTEM_LIBRARIES:=0}
if [ ${USE_UBUNTU_SYSTEM_LIBRARIES} == 1] ; then
...|
Thanks for your comments @KrisThielemans . Indeed my aim was to split #937 in subtasks, but I forgot to account for the VM, for instance. I was aiming at fixing docker with a second PR, keeping these separated. As we use the same build scripts for docker, VM and straight Ubuntu systems (the VM is one of them), we need to be able to differentiate what to install and what not. I understand now that I should not have broken backward compatibility. Probably, I should have added a variable that is |
that's a good idea. We could default it to 0 now, and switch to 1 later. But maybe you want to do that in the 2nd PR anyway (in which case this PR would be really small). As you prefer. |
not appropriate for this PR
KrisThielemans
left a comment
There was a problem hiding this comment.
Please squash-merge (with edited commit message) when GHA is happy
Updates the GHA to explicitly use Ubuntu 22.04.
Adds the
zenodo_getdependency inrequirements.txt.