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

[feature] Proper MSVC Building without needing MINGW #1424

Open
a-michelis opened this issue Sep 6, 2024 · 6 comments · May be fixed by #1440
Open

[feature] Proper MSVC Building without needing MINGW #1424

a-michelis opened this issue Sep 6, 2024 · 6 comments · May be fixed by #1440

Comments

@a-michelis
Copy link

Hello, everyone,

I attempted to build the project for native MSVC without success -as expected.
That said, I discovered that the only reason for this is the lack of proper MSVC handling on cmake/modules/Findlibusb.cmake.

In more detail, the original snippet for MSVC handling is the following:

        else (MSVC)
            # libusb library
            set(LIBUSB_NAME libusb-1.0)
            find_library(LIBUSB_LIBRARY
                NAMES ${LIBUSB_NAME}
                HINTS ${LIBUSB_WIN_OUTPUT_FOLDER}/MinGW${ARCH}/dll
                NO_DEFAULT_PATH
                NO_CMAKE_FIND_ROOT_PATH
                )
        endif()

If handled correctly, STLink library, as well as all tools are built properly (with some warnings, ofcourse) -at least with VS2022 which i tried.

index 15afb00..e14993b 100644
--- a/cmake/modules/Findlibusb.cmake
+++ b/cmake/modules/Findlibusb.cmake
@@ -107,12 +107,28 @@ elseif (WIN32 OR (MINGW AND EXISTS "/etc/debian_version"))      # Windows OR cro
         else (MSVC)
             # libusb library
             set(LIBUSB_NAME libusb-1.0)
+
+            if(140 EQUAL ${MSVC_TOOLSET_VERSION})
+                set(VSVERSION "2015")
+            elseif(141 EQUAL ${MSVC_TOOLSET_VERSION})
+                set(VSVERSION "2017")
+            elseif(142 EQUAL ${MSVC_TOOLSET_VERSION})
+                set(VSVERSION "2019")
+            elseif(143 EQUAL ${MSVC_TOOLSET_VERSION})
+                set(VSVERSION "2022")
+            else()
+                set(VSVERSION "2013")
+            endif()
+
             find_library(LIBUSB_LIBRARY
                 NAMES ${LIBUSB_NAME}
-                HINTS ${LIBUSB_WIN_OUTPUT_FOLDER}/MinGW${ARCH}/dll
+                HINTS ${LIBUSB_WIN_OUTPUT_FOLDER}/VS${VSVERSION}/MS${ARCH}/static
                 NO_DEFAULT_PATH
                 NO_CMAKE_FIND_ROOT_PATH
                 )
+
+            #fix for ill-defined ssize_t in basetsd.h
+            add_compile_definitions(_SSIZE_T_DEFINED ssize_t=int64_t)
         endif()
     endif()

The structure of 3rd party .7z contains different releases for different versions of VS. For that reason, the patch discovers the proper release to use (see the big if-elseif-else cascade).

Since it seems perfectly doable but I believe I lack basic cmake knowledge to consider the patch above decent, I share it with you to further discuss it and potentially merge it to the code!

Cheers,
A.M.

@Nightwalker-87
Copy link
Member

@a-michelis: Looking at your personal branch, I see you are currently working on this topic. I am open to improvements here, as it definitely needs some rework and updates. We should drop compatibility with now unsupported versions of VS as well. I'm not happy with this MinGW stuff inherited from earlier days. So far I just tried to keep it working at best effort and still we had some compilation problems on Windows with it in the past...

@a-michelis
Copy link
Author

a-michelis commented Oct 3, 2024

My fork is a kind-of big refactor -it attempts to give a more library-oriented approach to the project. Instead of grabbing the release .7z for libusb, it directly clones and uses the community-maintained libusb-cmake wrapper, allowing cmake to also build libusb tailored to the system's needs. I haven't touched the code, except from :

  • refactoring headers to be included as system (which later helps when the library is being used) and moving them to an stlink subfolder (so usage is currently #include <stlnk/%HEADER%.h>
  • some additional extern "C" blocks to export more useful functionality from the library.

The project builds successfully for :

  • windows (MSVC, VS2022),
  • wsl (Ubuntu 22.04, default GCC)
  • a somewhat old SUSE-based distro (to ensure compatibility with older kernels).

All mentioned distros did not have libusb installed -stlink downloaded and built it from scratch

I've used it in small, demo-like projects:

  • enumeration 🆗
  • connection to a usb device 🆗
  • resetting 🆗
  • force-debugging (essentially pause) 🆗
  • and running normally (essentially resume) 🆗
  • flash erashing 🆗
  • flash programming via byte array (mwrite variant) 🆗

All of these work properly with stlink-v3 (stm32h753zi nucleo) and stlink-v2 (stm32f401re nucleo).

My testing was by no means exhaustive, as you can observe.

Do you believe such refactor -or part of it, ofcourse- can be included in an iterative manner (aka next version(s))?

@Nightwalker-87
Copy link
Member

Nightwalker-87 commented Oct 11, 2024

Yes - of course, as long as there is some further testing. I am somehow familiar with the cmake building used for compilation. Nevertheless one should keep an eye on that intended iterative changes do not break anything as long as the current implementation remains present. Anyway, that's what the testing branch is intended for.

@Nightwalker-87 Nightwalker-87 pinned this issue Oct 17, 2024
@Nightwalker-87
Copy link
Member

@a-michelis: Can you provide a PR (as a draft) to allow for an initial review?

@Nightwalker-87 Nightwalker-87 moved this from In progress to In review in Release v1.8.1 Oct 19, 2024
@a-michelis
Copy link
Author

I apologize for the delayed response - my current IRL situation is a bit busy.

Of course, I can totally do that - it is not ready for a merge of any kind, but, as you said, it's a great way to gauge the work needed to normalize the changes and incorporate them appropriately.

@Nightwalker-87
Copy link
Member

I've reviewed the related PR. Looks good to me.
Now it's time for testing. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment