modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
453 stars 165 forks source link

Rebuild binaries for Windows (MSVC, TDM-GCC) and Linux (GCC) #4250

Closed beutlich closed 4 months ago

beutlich commented 6 months ago

I rebuilt the binaries using

to fix #4178.

beutlich commented 6 months ago

Should probably wait for #3700.

beutlich commented 6 months ago

Is that done on purpose?

Of course. I even purchased VS 2005 (on MA budget and being the first MSVC compiler targeting 64-bit architectures) for that reason ;-). The reason is, that all newer compilers are still object compatible to these old static libraries, i.e., they can link them w/o caring on C runtime issues. And as it worked out the last years: Never change an old compiler.

HansOlsson commented 6 months ago

Is that done on purpose?

Of course. (I even purchased VS 2005 (on MA budget) for that reason ;-)). The reason is, that all newer compilers are still object compatible to these old static libraries, i.e., they can link them w/o caring on C runtime issues. And as it worked out the last years: Never change an old compiler.

Yes, for C with at least two potential problems regarding the C runtime if linking with a newer compiler:

beutlich commented 6 months ago

Should probably wait for #3700.

This one is merged.

Still waiting for #4274 before rebuilding again.

beutlich commented 6 months ago

Still waiting for #4274 before rebuilding again.

Rebuilt with all desired PRs merged.

beutlich commented 5 months ago

While this is not yet reviewed/merged we can wait for #4278.

HansOlsson commented 5 months ago

While this is not yet reviewed/merged we can wait for #4278.

I will wait with reviewing until that is ready (and will then check that the libraries link etc).

maltelenz commented 5 months ago

I also want to bring up the alternative to just not have binaries in the repository. I strongly prefer that.

In general, I find it is a bad idea to have build products checked in into source repositories, because of reasons probably clear (often out of date, bloating the repository size, ...).

I expect all tool vendors build their own binaries anyway.

For CI, we need to build binaries for each run anyway, or the CI results will not mean much, since they will not match what is in the repository (when the C source changes).

The new CMake project proposal should help with CI efforts: #4265

If we want to provide binaries for users that don't want to use a tool-shipped version (even though you would expect it to be somewhat broken), maybe binaries could be built as part of the release process and included in the release zip file?

HansOlsson commented 5 months ago

I also want to bring up the alternative to just not have binaries in the repository. I strongly prefer that.

In general, I find it is a bad idea to have build products checked in into source repositories, because of reasons probably clear (often out of date, bloating the repository size, ...).

I generally agree (with some exceptions that don't apply here).

However, the problem is that we also want to test 4.1.0 before it is released, and without the binaries some testers will have a problem with that (as we have seen very clearly) - so I'm not sure if changing the process now is a good idea.

I can see two possibilities:

maltelenz commented 5 months ago

Maybe @GallLeo can comment on what they need for their regression test runs, and what they think of the procedure proposed by @beutlich here.

beutlich commented 5 months ago

CI is the way to go: Have push and PR triggered action to build the binaries and keep the artifacts.

GallLeo commented 5 months ago

For our regression test server, we manually built from source. Last week, according to #4265.

I.e. we either have to automize the compiler call on our side or use the built artifacts from Github CI. I think, only having binaries available for release builds (zip) is not enough.

maltelenz commented 5 months ago

For our regression test server, we manually built from source. Last week, according to #4265.

I.e. we either have to automize the compiler call on our side or use the built artifacts from Github CI.

I think this should be the way forward, for the reasons already brought up earlier in this thread.

However, if accepting new binaries in the repo for now will help with moving the release of 4.1.0 forward more smoothly, I don't mind.

Maybe we can remove the binaries on master to make clear that we don't want binaries in the repo going forward? I'd be happy to provide such a pull request.

beutlich commented 5 months ago

I'd be happy to provide such a pull request.

Yes, please go ahead.

beutlich commented 5 months ago

Last week, according to #4265.

@GallLeo I'd appreciate to get your/LTX review on that #4265 PR. Thanks.

beutlich commented 4 months ago

Waiting for #4345 to be merged.

beutlich commented 4 months ago

Rebased and rebuilt once again.

beutlich commented 4 months ago

@HansOlsson Note, that we do not need to wait for #4302 (since ModelicaInternal.c is compiled for libModelicaExternalC).

beutlich commented 4 months ago

As stated in #4347, there is no need to have the binaries in the repo - as long as they are part of the released asset. (Unfortunately this was not the case for the lastest pre-relases https://github.com/modelica/ModelicaStandardLibrary/releases/download/v4.1.0-alpha.1/ModelicaStandardLibrary_v4.1.0-alpha.1.zip or https://github.com/modelica/ModelicaStandardLibrary/releases/download/v4.1.0-beta.1/ModelicaStandardLibrary_v4.1.0-beta.1.zip.)

HansOlsson commented 4 months ago

As stated in #4347, there is no need to have the binaries in the repo - as long as they are part of the released asset. (Unfortunately this was not the case for the lastest pre-relases https://github.com/modelica/ModelicaStandardLibrary/releases/download/v4.1.0-alpha.1/ModelicaStandardLibrary_v4.1.0-alpha.1.zip or https://github.com/modelica/ModelicaStandardLibrary/releases/download/v4.1.0-beta.1/ModelicaStandardLibrary_v4.1.0-beta.1.zip.)

Ok, but the logic would be that we remove the libraries at some point in the future - as long as they exist they should be up-to-date.

Harisankar-Allimangalath commented 3 months ago

@casella @GallLeo @HansOlsson we have to merge this to the master also right , or is it already updated in the master ?

just for understanding , Thankyou

beutlich commented 3 months ago

No, this is not meant for bringing to master, see #4347.

casella commented 4 weeks ago

@Harisankar-Allimangalath we'll need to sort out the binary issue for 4.2.0, but that will be after the 4.1.0 release.