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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Mar 13, 2024
Merged

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

merged 1 commit into from
Mar 13, 2024

Conversation

beutlich
Copy link
Member

I rebuilt the binaries using

  • MSVC VS 2005
  • GCC (TDM64-1) 5.1.0
  • Linux GCC 4.8.5

to fix #4178.

@beutlich beutlich added the L: Resources Issue addresses Modelica/Resources (excl. C-Sources) label Jan 10, 2024
@beutlich beutlich added this to the MSL4.1.0 milestone Jan 10, 2024
@beutlich beutlich added the V: 4.1.0-dev Issue originates in MSL v4.1.0-dev (and is not present in earlier releases) label Jan 10, 2024
@beutlich beutlich enabled auto-merge (squash) January 10, 2024 20:34
@AHaumer AHaumer removed their request for review January 11, 2024 07:05
@beutlich beutlich mentioned this pull request Jan 12, 2024
Merged
@m-kormann m-kormann mentioned this pull request Jan 12, 2024
Merged
@beutlich beutlich mentioned this pull request Jan 14, 2024
Merged
@beutlich
Copy link
Member Author

Should probably wait for #3700.

@beutlich beutlich marked this pull request as draft January 15, 2024 19:20
@beutlich beutlich mentioned this pull request Jan 16, 2024
Closed
@beutlich beutlich changed the title Rebuild binaries Rebuild binaries for Windows (MSVC, TDM-GCC) and Linux (GCC) Jan 16, 2024
casella
casella reviewed Jan 16, 2024
View reviewed changes
Copy link
Contributor

@casella casella left a comment

Choose a reason for hiding this comment

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

@beutlich, the versions of the compilers that you used to build the binaries are really old. Is that done on purpose? Do we have established guidelines to produce those binaries?

Thanks!

@beutlich
Copy link
Member Author

beutlich commented Jan 16, 2024 •
edited
Loading

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
Copy link
Contributor

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:

  • Don't directly use stdio, stdout, and stderr between different compiler versions (VS 2015 was the breaking change). You can use printf, scanf, getchar etc; it's just the names stdio, stdout, and stderr that shouldn't be used.
  • Don't use macros to access the locale data-structure generated by a different compiler version. VS 2012 was the breaking change as far as I recall.

@beutlich
Copy link
Member Author

Should probably wait for #3700.

This one is merged.

Still waiting for #4274 before rebuilding again.

@beutlich beutlich force-pushed the update-binaries branch 3 times, most recently from 9858af0 to 3f8cad2 Compare January 18, 2024 20:21
@beutlich beutlich marked this pull request as ready for review January 18, 2024 20:21
@beutlich
Copy link
Member Author

Still waiting for #4274 before rebuilding again.

Rebuilt with all desired PRs merged.

@beutlich
Copy link
Member Author

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

@beutlich beutlich marked this pull request as draft January 22, 2024 19:56
HansOlsson
HansOlsson approved these changes Jan 26, 2024
View reviewed changes
Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Looking good.
I noticed two unrelated issues - will open PR for that.

@GallLeo
Copy link
Contributor

GallLeo commented Feb 2, 2024

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.

GallLeo
GallLeo approved these changes Feb 2, 2024
View reviewed changes
Copy link
Contributor

@GallLeo GallLeo left a comment

Choose a reason for hiding this comment

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

As we sucessfully ran nightly tests with the same source base, I assume these binaries are fine, as well.

@maltelenz
Copy link
Contributor

maltelenz commented Feb 2, 2024 •
edited
Loading

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
Copy link
Member Author

beutlich commented Feb 2, 2024

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

Yes, please go ahead.

@beutlich
Copy link
Member Author

beutlich commented Feb 2, 2024

Last week, according to #4265.

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

@beutlich
Copy link
Member Author

Waiting for #4345 to be merged.

maltelenz added a commit to maltelenz/ModelicaStandardLibrary that referenced this pull request Feb 28, 2024
@maltelenz
Remove binaries from repository.
3b8bb31 
Binaries should be built separately, as discussed in modelica#4250.
@maltelenz maltelenz mentioned this pull request Feb 28, 2024
Open
@beutlich beutlich force-pushed the update-binaries branch 2 times, most recently from ecc8ded to ddc5267 Compare February 28, 2024 20:07
@beutlich
Copy link
Member Author

Rebased and rebuilt once again.

@tobolar tobolar mentioned this pull request Mar 1, 2024
Open
15 tasks
@beutlich
Rebuild binaries
05c03a6 
* MSVC VS 2005
* GCC (TDM64-1) 5.1.0
* Linux GCC 4.8.5
@beutlich beutlich marked this pull request as ready for review March 12, 2024 18:13
@beutlich
Copy link
Member Author

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

@beutlich
Copy link
Member Author

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
Copy link
Contributor

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.

@HansOlsson HansOlsson merged commit 90d43f3 into modelica:maint/4.1.0 Mar 13, 2024
2 checks passed
@beutlich beutlich deleted the update-binaries branch March 13, 2024 14:51
@beutlich beutlich mentioned this pull request Mar 13, 2024
Closed
@Harisankar-Allimangalath
Copy link
Contributor

@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
Copy link
Member Author

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

@casella
Copy link
Contributor

casella commented Jun 18, 2024

@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.

beutlich pushed a commit to maltelenz/ModelicaStandardLibrary that referenced this pull request Oct 1, 2024
@maltelenz @beutlich
Remove binaries from repository.
0f40a64 
Binaries should be built separately, as discussed in modelica#4250.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@casella casella casella left review comments

@GallLeo GallLeo GallLeo approved these changes

@HansOlsson HansOlsson HansOlsson approved these changes

Assignees

@casella casella

@HansOlsson HansOlsson

Labels
L: Resources Issue addresses Modelica/Resources (excl. C-Sources) V: 4.1.0-dev Issue originates in MSL v4.1.0-dev (and is not present in earlier releases)
Projects
None yet
Milestone
MSL4.1.0
Development

Successfully merging this pull request may close these issues.

Recompilation of binaries necessary
6 participants
@beutlich @HansOlsson @maltelenz @GallLeo @Harisankar-Allimangalath @casella