precice / calculix-adapter

preCICE-adapter for the CSM code CalculiX
GNU General Public License v3.0
52 stars 20 forks source link

Automatic creation of .deb packages #72

Closed boris-martin closed 2 years ago

boris-martin commented 3 years ago

This adds a Github Actions workflow that download dependencies and Calculix source code, builds the adapter (the "regular" version, without PaStiX), creates a .deb package and uploads it as artifact. (It can then be downloaded from Github on the workflow run) Currently runs on Ubuntu 18.04 and 20.04 (The 2 latest LTS versions). It should work on other versions but I couldn't get Github to run on 21.04, and it stalled the entire workflow forever so I removed it.

Usage : download the corresponding artifact and sudo apt install [packagefile]. This requires an installation of preCICE through the provided packages (i.e. this package won't work with a custom build of preCICE, as it explicitly requires the libprecice package)

The makefile has been updated to be similar (just replaced 2.16 by 2.17 everywhere) to the one of the master branch instead of the current v2.17, which is outdated and painful to use, especially within scripts.

Things yet to do :

Things to do or think about in the long run :

fsimonis commented 2 years ago

The following is a good read regarding the license compatibility: https://www.gnu.org/licenses/license-compatibility.html

CalculiX is licensed under the GPLv2 license, which requires any redistributed versions to be licensed under the same license. So, we have an issue.

boris-martin commented 2 years ago

The following is a good read regarding the license compatibility: https://www.gnu.org/licenses/license-compatibility.html

CalculiX is licensed under the GPLv2 license, which requires any redistributed versions to be licensed under the same license. So, we have an issue.

Can't we simply have the adapter be GPL2 instead of GPL3 ? That looks like the best way to remove headaches to me. Unless we can't because we'd need the authorization of all people who contributed to the adapter or something like this ?

MakisH commented 2 years ago

Currently runs on Ubuntu 18.04 and 20.04 (The 2 latest LTS versions). It should work on other versions but I couldn't get Github to run on 21.04, and it stalled the entire workflow forever so I removed it.

Where did it hang with 21.04? Did you get any error messages?

Usage : download the corresponding artifact and sudo apt install [packagefile]. This requires an installation of preCICE through the provided packages (i.e. this package won't work with a custom build of preCICE, as it explicitly requires the libprecice package)

We would then also need to add this information to the adapter documentation.

The makefile has been updated to be similar (just replaced 2.16 by 2.17 everywhere) to the one of the master branch instead of the current v2.17, which is outdated and painful to use, especially within scripts.

Can we just merge this change everywhere needed, please? :-) This is anyway what we have currently documented.

* The package has version "2.17-1" and it's hardcoded, I don't know if we should number the versions according to CCX or to preCICE.

We need a versioning scheme here and we should put some thought on it. Since the adapter works with only specific versions of CalculiX, but any preCICE v2.x, I would suggest something like calculix-precice2-2.17-1.

* I know the preCICE packages has security stuff like a sha256 key, but I have zero knowledge about it, so currently the package doesn't feature this.

You can generate such a hash very easily, with sha256sum <file>. This is mainly to certify that the package has not been modified.

* Building the adapter with PaStiX is much harder, and I spent too many hours trying to do it automatically. (For some reasons, I miss some CUDA related dependencies on Github Actions despite installing the required package, though I have no issues with my own VMs). Furthermore, PaStiX requires a lot of hand-built libraries (with specific flags and so on), so integrating it with the Ubuntu ecosystem is impossible, and we'd have to package the dependencies too, and... Well, it looks like a nightmare.

Let's forget about PaStiX in the Debian package right now. This PR is already a significant step ahead.

* Once the packages are made, we still have to release them.

We should eventually also start making releases here, similarly to other adapters. This would be much better than the current branching model.

Can't we simply have the adapter be GPL2 instead of GPL3 ? That looks like the best way to remove headaches to me. Unless we can't because we'd need the authorization of all people who contributed to the adapter or something like this ?

As we already discussed on Gitter, maybe this is indeed what we should do. Reaching out to all contributors should be still doable. But let's make sure first that this is the right thing to do.

boris-martin commented 2 years ago

Currently runs on Ubuntu 18.04 and 20.04 (The 2 latest LTS versions). It should work on other versions but I couldn't get Github to run on 21.04, and it stalled the entire workflow forever so I removed it.

Where did it hang with 21.04? Did you get any error messages?

No errors, just infinite waiting. Maybe they have already switched everything to 21.10 ?

Usage : download the corresponding artifact and sudo apt install [packagefile]. This requires an installation of preCICE through the provided packages (i.e. this package won't work with a custom build of preCICE, as it explicitly requires the libprecice package)

We would then also need to add this information to the adapter documentation.

Yes, I was thinking of waiting for the official release.

The makefile has been updated to be similar (just replaced 2.16 by 2.17 everywhere) to the one of the master branch instead of the current v2.17, which is outdated and painful to use, especially within scripts.

Can we just merge this change everywhere needed, please? :-) This is anyway what we have currently documented.

I'm all in favor, but I'd prefer someone more experienced handled that. (Not really changing the makefile, but 2.17 branch is already 45 commits behind master, so that's slightly messy)

* The package has version "2.17-1" and it's hardcoded, I don't know if we should number the versions according to CCX or to preCICE.

We need a versioning scheme here and we should put some thought on it. Since the adapter works with only specific versions of CalculiX, but any preCICE v2.x, I would suggest something like calculix-precice2-2.17-1.

By the way, I'm not 100% sure what preCICE version to use for the build. If we compile with 2.3 but use only features from <= 2.2, would the adapter work on a computer with preCICE 2.2 installed, for instance ? More generally, should we always build with the most recent version of preCICE 2.x ?

* I know the preCICE packages has security stuff like a sha256 key, but I have zero knowledge about it, so currently the package doesn't feature this.

You can generate such a hash very easily, with sha256sum <file>. This is mainly to certify that the package has not been modified.

So we simply need to provide the output of sha256sum so that people can check ?

* Building the adapter with PaStiX is much harder, and I spent too many hours trying to do it automatically. (For some reasons, I miss some CUDA related dependencies on Github Actions despite installing the required package, though I have no issues with my own VMs). Furthermore, PaStiX requires a lot of hand-built libraries (with specific flags and so on), so integrating it with the Ubuntu ecosystem is impossible, and we'd have to package the dependencies too, and... Well, it looks like a nightmare.

Let's forget about PaStiX in the Debian package right now. This PR is already a significant step ahead.

That seems very reasonable to me.

* Once the packages are made, we still have to release them.

We should eventually also start making releases here, similarly to other adapters. This would be much better than the current branching model.

Even with clean releases, there's still the issue of having different code for each CalculiX version no ?

MakisH commented 2 years ago

No errors, just infinite waiting. Maybe they have already switched everything to 21.10 ?

21.04 is still supported (and I am still using it).

By the way, I'm not 100% sure what preCICE version to use for the build. If we compile with 2.3 but use only features from <= 2.2, would the adapter work on a computer with preCICE 2.2 installed, for instance ? More generally, should we always build with the most recent version of preCICE 2.x ?

We should build/test with the latest version of preCICE. But, because we are linking to a shared library, any preCICE v2.x version will work. (there was even an AdvProg slide on lecture "Build time" regarding exactly this example :sweat_smile: )

So we simply need to provide the output of sha256sum so that people can check ?

Exactly. Unless linter suggests more steps.

Even with clean releases, there's still the issue of having different code for each CalculiX version no ?

Yes. But we should have a clear policy on which CalculiX versions we support. I would vote to only support the latest one.

boris-martin commented 2 years ago

No errors, just infinite waiting. Maybe they have already switched everything to 21.10 ?

21.04 is still supported (and I am still using it).

Weird, I should give it another try at some point.

By the way, I'm not 100% sure what preCICE version to use for the build. If we compile with 2.3 but use only features from <= 2.2, would the adapter work on a computer with preCICE 2.2 installed, for instance ? More generally, should we always build with the most recent version of preCICE 2.x ?

We should build/test with the latest version of preCICE. But, because we are linking to a shared library, any preCICE v2.x version will work. (there was even an AdvProg slide on lecture "Build time" regarding exactly this example 😅 )

Woops :D

So we simply need to provide the output of sha256sum so that people can check ?

Exactly. Unless linter suggests more steps.

As far as I remember it didn't complain about the lack of sha256, I was more thinking about how we just publish keys every time there's a new preCICE release.

Even with clean releases, there's still the issue of having different code for each CalculiX version no ?

Yes. But we should have a clear policy on which CalculiX versions we support. I would vote to only support the latest one.

Probably a good idea to add support for 2.18 before dropping the others then !

boris-martin commented 2 years ago

Slightly unrelated, but following this https://github.com/precice/precice/pull/1132#discussion_r746697201 I realized it was probably interesting (in the long run) to modify Makefiles to provice a make instal which includes a few things from here, like man pages, copyright and changelog.

MakisH commented 2 years ago

Slightly unrelated, but following this precice/precice#1132 (comment) I realized it was probably interesting (in the long run) to modify Makefiles to provice a make instal which includes a few things from here, like man pages, copyright and changelog.

I am not sure if investing on Make would be the right thing to do in that direction. Migrating to CMake could open such possibilities, but I guess it would be complicated to do it in the downstream project (adapter) without already having support from the upstream (CalculiX + dependencies).

MakisH commented 2 years ago

Regarding licensing, this is the license of CalculiX, as found in the CalculiX.h of version 2.17:

/*     CALCULIX - A 3-dimensional finite element program                 */
/*              Copyright (C) 1998-2020 Guido Dhondt                     */

/*     This program is free software; you can redistribute it and/or     */
/*     modify it under the terms of the GNU General Public License as    */
/*     published by the Free Software Foundation; either version 2 of    */
/*     the License, or (at your option) any later version.               */

/*     This program is distributed in the hope that it will be useful,   */
/*     but WITHOUT ANY WARRANTY; without even the implied warranty of    */ 
/*     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the      */
/*     GNU General Public License for more details.                      */

/*     You should have received a copy of the GNU General Public License */
/*     along with this program; if not, write to the Free Software       */
/*     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.         */

Since we have the either version 2 of the License, or (at your option) any later version., I would say we have no problem with GPLv3.

From the FSF article that @fsimonis posted above:

The FSF uses the approach of asking people to release programs under “GNU GPL version N or any later version.” This licensing is compatible with version N, and also with N+1 (because it offers version N+1 as an option). When you combine code under “GPL 3 or later” with code under “GPL 2 or later”, the license of the combination is their intersection, which is “GPL 3 or later”.

The correct license of calculix-precice, as a code derived from CalculiX, and given our additions as GPLv3, is GPLv3.

In our case, we maintain the GPLv2 notice in CalculiX.h, but also provide GPLv3 for our code. I think this is ok but again, I barely understand licensing.

boris-martin commented 2 years ago

I did some improvements :

I'm undrafting the PR, though I also plan some adjustements to packagin/README.md

boris-martin commented 2 years ago

@MakisH @KyleDavisSA I don't have the rights to fix the conflict, but it's a trivial one. After this I think it's ready to merge

boris-martin commented 2 years ago

It works on my working Ubuntu VM, worked on my Mint VM before this VM died, and on a clean WSL Ubuntu 20.04. I think tests are conclusive :)

uekerman commented 2 years ago

Nice! Please don't forget to also add this to the adapter documentation.

MakisH commented 2 years ago

Nice! Please don't forget to also add this to the adapter documentation.

Indeed, but we should probably start making releases first.