ERGO-Code / HiGHS

Linear optimization software
MIT License
957 stars 177 forks source link

No version info on binaries in Windows #1610

Closed peno64 closed 5 months ago

peno64 commented 8 months ago

Under Windows, each exe and dll should have the version imbeded in the binary. This is done via a resource file used when compiling/linking the binaries. Then when the properties of the binary is retrieved, it should show this version. But even more. You can see a copyright and a company name also but the version is the most important and this when the application is distributed with an installer. Windows installers look at this version number to determine if a binary must be overwritten. If the version is older or equation, it will not be overwritten. When the version is newer is will be overwritten/updated. I am not sure what happens if the binary has no version. Either it will always be overwritten or not.

The problem is now that HiGHS does not set the version information on its binaries. This is not on the precompiled binaries on JuliaBinaryWrappers and also not if you compile the binaries yourself (https://github.com/ERGO-Code/HiGHS/blob/master/docs/src/installation.md or https://github.com/ERGO-Code/HiGHS/issues/694)

Can this be added in future versions?

jajhall commented 8 months ago

I restored the printing of the version number, but I can't see when. It's in v1.6.0

Since 1.6.0 HiGHS - optionally - logs the githash.

As for "resource files" when creating the binaries, I have no idea. Can you comment @odow or @galabovaa ?

odow commented 8 months ago

I don't use Windows so I don't know what this is or what it would require.

What is the aforementioned Windows installer? HiGHS is just an executable?

peno64 commented 8 months ago

HiGHS is hardly "just an executable" ;-)

HiGHS can also be compiled as a shared library which is a dll under Windows. You can use this dll in an application to optimize your model via the API. To provide the application to a user, the program among with dlls are installed at the users machine with an installer and it is that installer that checks version numbers. Therefore it is important that the dll has a version number such that the installer can check if a newer version of the dll is provided and thus if it must be replaced or not.

jajhall commented 8 months ago

You make your point. How we are able to give the dll a version number is another question. I've no idea

Coloquinte commented 8 months ago

There are some answers on stackoverflow on how to do this on Windows, so this might be the way, but this will be a Windows-specific wart. There are examples online, such as Protobuf.

A simpler approach may be to use the VERSION property in Cmake for the executable in app/CMakeLists.txt, as is already done for the shared library. It claims to work for executables, but no additional information is given.

galabovaa commented 7 months ago

Can you please check if this has been resolved for the binaries you compile yourself in our latest version 1.7.0 in master? Thanks to a contribuition by @Coloquinte

peno64 commented 7 months ago

@galabovaa

Thank you for looking into this. Unfortunately, the highs dll is not build anymore with the latest master version.

I use the following commands to build everything:

mkdir build cd build cmake -DFAST_BUILD=off .. cmake --build . --config Release

in RELEASE\bin (I think this was bin\release before) I get:

HighsCsharp.dll highs.lib capi_unit_tests.exe capi_unit_tests.pdb csharpexample.exe highs.exe highs.pdb unit_tests.exe unit_tests.pdb

but no highs.dll or libhighs.dll

highs.exe does not have a version number.

These are the file in folder RELEASE\bin after bul

peno64 commented 7 months ago

When using branch dd4a9a2a from 29/01/2024, so a branch before your two PR from 7/3/2024 The following is build under bin\Release:

capi_unit_tests.exe csharpexample.exe highs.dll highs.exe HighsCsharp.dll unit_tests.exe

So here highs.dll is still build and again note that it is now again in bin\Release and not in RELEASE\bin with the latest master

peno64 commented 7 months ago

When build with branch 50670fd4 the problem already occurs. now the binaries are in RELEASE\bin and the highs dll isn't there anymore. Also I get alot of compiler warnings with this. This is attached in buildhighs.txt Before this branch all these warnings do not occur. buildhighs.txt

galabovaa commented 7 months ago

Thank you for your quick response too!

Why do you have "FAST_BUILD=OFF"? Would you try to use ON, which is the default, and let me know if you get issues then?

FAST_BUILD was a flag we introduced to move from an outdated cmake approaches to a "more modern cmake". We've kept it as an option to avoid breaking applications relying on us during the refactoring and update of the build system, which is mostly done now. So it would be helpful for me if you could share if you have problems, so I can clean up any issues that FAST_BUILD ON may be causing for you.

One thing that we changed is the default behaviour on WIndows, building static libs by default. Would you also please add "-DBUILD_SHARED_LIBS=ON" to your cmake parameters and try again?

I will look at how to turn off the warnings. I made an attempt earlier, but I failed and decided to get the functionality going first.

Frankly, I have no idea which one of my many edits changed the build dir from bin\Release to RELEASE\bin. I was trying to follow what other packages have done, to respect Visual Studio. Do you have an idea which of the two is better / more standard on Windows?

peno64 commented 7 months ago

When I use:

cmake .. cmake --build . --config Release

The following files are build:

call_highs_from_c.exe call_highs_from_c.pdb call_highs_from_cpp.exe call_highs_from_cpp.pdb highs.exe highs.lib highs.pdb

So the following files are not build:

capi_unit_tests.exe csharpexample.exe HighsCsharp.dll

When I use: cmake -DBUILD_SHARED_LIBS=ON .. cmake --build . --config Release

I get:

call_highs_from_c.exe call_highs_from_c.pdb call_highs_from_cpp.exe call_highs_from_cpp.pdb highs.dll highs.exe highs.exp highs.lib highs.pdb

So the following files are still not build:

capi_unit_tests.exe csharpexample.exe HighsCsharp.dll

But highs.dll is now build. And it does have a version number, however it gives:

File version 1.0.0.0 Product version 1.7.0

File version and product version should be the same (1.7.0)

When I use

cmake -DBUILD_SHARED_LIBS=ON -DFAST_BUILD=off .. cmake --build . --config Release

I get:

capi_unit_tests.exe capi_unit_tests.pdb csharpexample.exe highs.dll highs.exe highs.exp highs.lib highs.pdb HighsCsharp.dll unit_tests.exe unit_tests.pdb

So now everything is build but highs.dll does not have the version info anymore.

bin\release (and bin\debug) are the more standard ones. Visual Studio uses that folder structure for both C and csharp programs.

Coloquinte commented 7 months ago

Working on it. Indeed I only implemented it for FAST_BUILD, and there is a replacement missing in FILEVERSION.

Coloquinte commented 7 months ago

Fixed now in my branch, and works with FAST_BUILD=OFF. Note again that the binary has no version info attached - if you want to, it's quite easy to do by copying the cmake code for the library, but I'm keeping my change small.

galabovaa commented 7 months ago

Thank you, @Coloquinte, for looking into this so soon, I've merged your branch in latest. I also added a version in the same way to the executable, and checked on my Windows that both look OK now.

@peno64 so you are using C#? I have just started looking into what is needed to package highs as a NuGet!

The set of unit tests is rather large, so we added an extra parameter to build all tests. It is set by -DALL_TESTS=ON. It only works with FAST_BUILD=ON.

Another thing that we have changed is the default behaviour for the cmake build: since many users won't need C# we have made it to be included optionally, with the CSHARP cmake option. FAST_BUILD=OFF is only kept for legacy code, so would you please try

cmake -DCSHARP=ON ..

This flag enables BUILD_SHARED_LIBS within CMake, so you don't need to specify it explicitly.

If you want to have the tests, go with

cmake -DCSHARP=ON -DALL_TESTS=ON ..

I have also switched off the warnings for msvc and changed the dir back to bin/Debug and bin/Release, thank you for your tip.

Please let me know if you get the versions right and if you see any further issues!

galabovaa commented 7 months ago

Perhaps I should include the cmakeexample test with the default quick tests, when CSHARP is on. I have just realised this and will add that to latest with another PR, since it might require more edits to cmake.

peno64 commented 7 months ago

@galabovaa

I use C# but not the C# dll. I access the C dll in my C# code. There are several reasons for that.

I have no problem with the new compiler flags as long as it is all documented somewhere.

galabovaa commented 7 months ago

It will be documented properly with upcoming releases. So far, our top priority has been getting things to work. Please hang on, we'll make it to the documentation sooner rather than later.

Any suggestions or comments on how to improve our C Sharp dll would be much appreciated too.

peno64 commented 7 months ago

Thank you.

About not using the C Sharp dll, this has nothing to do with the dll itself. I made my own interface between the CSharp application and the solvers that can be used. Solvers in that it can use lpsolve, highs, scip, gurobi and Lindo and for the application all the same interface is used so it is just a switch to use another solver. The layer in between makes the translation to the API of the solver. The C dll is used because it always has the most complete functionality while another wrapper in between, in your case a CSharp dll, will most probably only provide a limited functionality and since we already have our wrapper there is no point in going again trough a second wrapper.

galabovaa commented 5 months ago

This should be resolved now. Please re-open if you run into further issues.