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 on Windows #6740

Closed

Conversation

CoolSpy3
Copy link
Contributor

@CoolSpy3 CoolSpy3 commented Dec 21, 2024

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

I did a quick check between the autogenerated and manually written files, and it looks like the new ones are the same, with two distinctions:

  1. The manually generated files only listed symbols that Webots explicitly added. The new ones list every symbol in the dll. This may lead to marginally larger files and the ability to link to previously private members of the dll, but I don't see either of those being a problem.
  2. The autogenerated files contain address information for each symbol. This was not present in the old files. If anything, this might just make the lib files more closely coupled to the dlls. Given that they are distributed together, this also shouldn't be a problem.

My knowledge of the MSVC toolchain is somewhat limited, so take the above with a grain of salt.

The other major note is that the Controller.def file is currently used to verify the Matlab function set. My guess is that I can rewrite the code to filter out just the webots definitions from the generated files (because they start with wb), but it will probably be a bit less cut-and-dry than the old implementation.

UPDATE: I've gone through and done a more detailed comparison between the old and new versions of the Controller.def files. Here's a rundown of the differences:
The old version has an additional 5 functions listed:
wb_range_finder_image_get_depth - This is actually a #define statement, so it's included purely in the header files. It does not need to be in the def.
wbr_camera_set_focal_distance and wbr_camera_set_fov - These functions are only used by the remote control plugin. They also do not need to be exported.
wbu_robot_window_configure and wbu_robot_window_update - These functions do not exist anywhere else in the codebase. My guess is that they were at some point removed, but the Controller.def file was never updated.

The new version has a bunch of additional functions. As stated above. Most of these are library functions and other statically-linked stuff. Here, I will ignore everything besides the Webots functions/constants (stuff starting with wb).

Here's a full list
wb_abstract_camera_cleanup
wb_abstract_camera_enable
wb_abstract_camera_get_fov
wb_abstract_camera_get_height
wb_abstract_camera_get_near
wb_abstract_camera_get_sampling_period
wb_abstract_camera_get_width
wb_abstract_camera_handle_command
wb_abstract_camera_new
wb_abstract_camera_write_request
wb_accelerometer_init
wb_altimeter_init
wb_brake_init
wb_brake_set_damping_constant_no_mutex
wb_camera_init
wb_camera_recognition_get_object
wb_compass_init
wb_connector_init
wb_device_cleanup
wb_device_init
wb_display_init
wb_distance_sensor_init
wb_emitter_init
wb_file_get_extension
wb_gps_init
wb_gyro_init
wb_inertial_unit_init
wb_joystick_init
wb_keyboard_init
wb_led_init
wb_lidar_get_point
wb_lidar_get_unique_id
wb_lidar_init
wb_light_sensor_init
wb_microphone_disable
wb_microphone_init
wb_motor_init
wb_motor_set_position_no_mutex
wb_mouse_get_state_pointer
wb_mouse_init
wb_pen_init
wb_position_sensor_init
wb_radar_get_target
wb_radar_init
wb_radio_disable
wb_radio_enable
wb_radio_event_get_data
wb_radio_event_get_data_size
wb_radio_event_get_emitter
wb_radio_event_get_radio
wb_radio_event_get_rssi
wb_radio_get_address
wb_radio_get_bitrate
wb_radio_get_channel
wb_radio_get_frequency
wb_radio_get_rx_sensitivity
wb_radio_get_tx_power
wb_radio_init
wb_radio_message_delete
wb_radio_message_get_body
wb_radio_message_get_destination
wb_radio_message_get_length
wb_radio_message_new
wb_radio_send
wb_radio_set_address
wb_radio_set_bitrate
wb_radio_set_callback
wb_radio_set_channel
wb_radio_set_frequency
wb_radio_set_rx_sensitivity
wb_radio_set_tx_power
wb_range_finder_get_unique_id
wb_range_finder_init
wb_receiver_init
wb_robot_flush_unlocked
wb_robot_get_step_duration
wb_robot_window_load_library
wb_skin_init
wb_speaker_init
wb_supervisor_init
wb_supervisor_load_world
wb_supervisor_save_world
wb_touch_sensor_init
wb_vacuum_gripper_init
wbr_abstract_camera_get_image_buffer
wbr_abstract_camera_set_image
wbr_altimeter_set_value
wbr_camera_recognition_set_object
wbr_gps_set_speed
wbr_gps_set_velocity_vector
wbr_lidar_get_image_buffer
wbr_lidar_set_image
wbr_radar_set_targets
wbr_range_finder_get_image_buffer
wbr_range_finder_set_image
wbu_system_tmpdir
wbu_system_webots_instance_path
Summary: wb_abstract_camera_* - These are used by the various implementations of cameras and are not usually called directly.
wb_radio_* - These are used by the radio plugin.
wbr_* - These are used by the remote control plugin.
wb_*_init`/`wb_*_cleanup - These are mostly called automatically by other parts of the API.
*_no_mutex - Variants of other functions that do not lock the robot mutex. Again, used internally. A couple of `wb_robot_* functions that are part of the internal API.
wb_(lidar/range_finder)_get_unique_id - Unused internal functions to get the unique id of the underlying abstract camera object for the corresponding node types.

wb_file_get_extension - Internal API function. wb_node_get_name - Only available in the C API.
A couple functions used internally by the Matlab API
wb_camera_recognition_get_object
wb_lidar_get_point
wb_mouse_get_state_pointer
wb_radar_get_target
Some functions that I think should've probably been included in the original Controller.def file
wb_supervisor_load_world
wb_supervisor_save_world
wbu_system_tmpdir
wbu_system_webots_instance_path

Related Issues
This pull-request fixes issue #6434 .

@CoolSpy3 CoolSpy3 added the test distribution Start the distribution test label Dec 21, 2024
@CoolSpy3 CoolSpy3 removed the test suite Start the test suite label Dec 25, 2024
@Kreijstal
Copy link
Contributor

just use the msys2 toolchain?

@CoolSpy3
Copy link
Contributor Author

just use the msys2 toolchain?

You're right that we could swap entirely to the MSVC toolchain on Windows, but that would necessitate writing and maintaining an entirely separate build path specifically for Windows as, atm, all platforms are built with gcc.

Additionally, because the same Makefiles are used to build user projects, we would then also have to ship the MSVC toolchain with the Windows Webots installation (or maintain a third, gcc-on-Windows build path), which is a whole other can of worms.

For those reasons, I would much prefer finding a way to get MSVC to generate the required files from the existing gcc-compiled files.

@Kreijstal
Copy link
Contributor

just use the msys2 toolchain?

You're right that we could swap entirely to the MSVC toolchain on Windows, but that would necessitate writing and maintaining an entirely separate build path specifically for Windows as, atm, all platforms are built with gcc.

Additionally, because the same Makefiles are used to build user projects, we would then also have to ship the MSVC toolchain with the Windows Webots installation (or maintain a third, gcc-on-Windows build path), which is a whole other can of worms.

For those reasons, I would much prefer finding a way to get MSVC to generate the required files from the existing gcc-compiled files.

I meant msys2 not msvc

@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Dec 27, 2024

I meant msys2 not msvc

Sorry, my bad, I misinterpreted your original comment.

From my understanding, msys2 is just a collection of Windows software ports; not a toolchain. AFAIK, the only major C/C++ toolchains are MSVC, Clang/LLVM, and GCC.

Webots currently uses the gcc toolchain (msys2's version on Windows). However, despite my best efforts, I've been unable to find a way to get gcc to generate MSVC style importlibs directly. Hence, this approach.

@Kreijstal
Copy link
Contributor

I meant msys2 not msvc

Sorry, my bad, I misinterpreted your original comment.

From my understanding, msys2 is just a collection of Windows software ports; not a toolchain. AFAIK, the only major C/C++ toolchains are MSVC, Clang/LLVM, and GCC.

Webots currently uses the gcc toolchain (msys2's version on Windows). However, despite my best efforts, I've been unable to find a way to get gcc to generate MSVC style importlibs directly. Hence, this approach.

msys2 is a distro that provides mingw which is gcc with libraries and headers to build on windows. msys2 provides the toolchain such as gcc, rust, go, mono, node, clang, etc.

Afaik gcc generates .a files, I think they're the same as .lib files... just rename them. But you can just use them with msvc (as long as they are C and not CPP)

@CoolSpy3
Copy link
Contributor Author

Afaik gcc generates .a files, I think they're the same as .lib files... just rename them. But you can just use them with msvc (as long as they are C and not CPP)

I just ran a few tests, and you appear to be right! I knew about those files, but I thought they were ELF rather than COFF. (I suppose that's my fault for not double-checking :P )

Thanks for the tip! There's still some more work I want to do to get these changes ready for review, but you just saved me a bunch of it :D

@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Jan 8, 2025

This PR has a lot of confusing commits and it looks like there's a much simpler path forward, so I've elected to redo these changes and split them across #6752 and #6753.

@CoolSpy3 CoolSpy3 closed this Jan 8, 2025
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
Development

Successfully merging this pull request may close these issues.

2 participants