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.

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

Build lib Files with gcc on Windows #6753

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

CoolSpy3
Copy link
Contributor

@CoolSpy3 CoolSpy3 commented Jan 8, 2025

Description
This PR supersedes #6740.

Currently, Windows import libraries (.lib files) are only being generated for some binaries. This makes it difficult to link to these binaries with the MSVC toolchain. This was previously done because generating these files requires listing all of there exports in a file, which had to be done by hand. This PR updates the relevant Makefiles to use MinGW's dlltool to generate these export listings. These can then be compiled to import libraries by MSVC.

This PR updates the relevant Makefiles to generate the libraries with gcc, which does not have the aforementioned requirement. (Thanks to @Kreijstal for the suggestion!) From my understanding, these should provide the same functionality as the visual-cpp-generated files, but with two added benefits:

  1. The generation is now fully automatic. This means that there is no need for new contributors to remember to export new symbols. This is especially helpful for the cpp library variants, which have name-mangled symbols. Atm, these have no explicit symbol listing, and thus, no generated import libraries.
  2. As far as I can tell, the generation of import libraries was the build process's only dependency on the visual-cpp build tools. This means that after this change, the Visual Studio dependencies can be removed from the Makefiles and GitHub Actions scripts.

Combined with #6752, this makes the def files throughout the repo completely obsolete. I've deleted them in this PR, however, because of their additional use in the MATLAB sources test, this will make the sources tests fail until #6752 is merged.

The removal of Visual Studio and def files from the build process will also necessitate that those elements are removed from the Wiki. I will make those changes after this PR is merged.

Related Issues
This pull-request fixes issue #6434.

Tasks
Add the list of tasks of this PR.

  • Rewrite the existing builds to generate their import libraries with gcc
  • Remove the outdated parts of the build (visual-cpp and *.def files)
  • Check the symbols exported by the new files against the old ones and note any significant changes in this PR.
  • Update the changelog

@CoolSpy3 CoolSpy3 added test distribution Start the distribution test test suite Start the test suite labels Jan 8, 2025
@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Jan 12, 2025

I've checked the new files against the old ones, the changes are pretty minimal for the most part.

car.lib

  • Now exports LIGHT_INDEX (an enum that's only accessible through private header files)
  • No longer exports _wbu_car_initialization_is_possible (A non-existant function)

driver.lib

  • Now exports LIGHT_INDEX (See above)
  • Now exports some deprecated methods (These methods are still listed in the header files, so I agree with gcc's choice here.)

Controller.lib

  • Now exports a bunch of functions from libraries and internal methods (see Build lib Files on Windows #6740 for details. Again, I believe this is okay.)
  • No longer exports wb_range_finder_image_get_depth (This is a macro and should not be exported)

CppController.lib

I'm not going to parse the name-mangled nonsense here, but the old version was generated by the same method so the changes should be irrelevant. A quick scan shows that some new methods were added (likely additions to the cpp api since R2023b [which is what I was testing against]), and some std functions were removed.

Here's a zip containing the old and new export lists and diffs between the relevant files.

@CoolSpy3 CoolSpy3 marked this pull request as ready for review January 12, 2025 06:56
@CoolSpy3 CoolSpy3 requested a review from a team as a code owner January 12, 2025 06:56
@lukicdarkoo
Copy link
Member

@CoolSpy3 We have almost finished QA for the new release. I would avoid merging any new contributions (except bug fixes) until the new release is out.

@CoolSpy3
Copy link
Contributor Author

Sounds good! Good luck on the next release 🚀 Can't wait :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test distribution Start the distribution test test suite Start the test suite
Development

Successfully merging this pull request may close these issues.

2 participants