Host software CMake cleanup: attempt #2#1016
Host software CMake cleanup: attempt #2#1016multiplemonomials wants to merge 20 commits intogreatscottgadgets:mainfrom
Conversation
|
Thank you so much for taking the time to work on this! We will review it soon. |
62f1b9d to
d8ac3b1
Compare
|
Had a chance to look at this yet? |
|
I'm planning to make a release shortly and then review and merge this PR right after the release. |
…ttgadgets#664)"" This reverts commit aa0485d.
…e root directory is
…d system (now that I understand what it does)
d8ac3b1 to
6cef11a
Compare
|
@mossmann Pinging again one year later, I rebased this to be compatible with latest master branch. Everything look ok to merge? I also updated the new github actions windows job to be compatible with this MR. Now, it uses Last but not least, I'm seeing something odd where my local clang-format and the CI job are disagreeing about how to format one specific block of code ( |
|
Thank you for updating this PR again! We really appreciate your patience. Our team is stretched very thin and will continue to be for a while. I'll do my best to see if I can get some community reviews and free up some time from the reviewers within GSG. |
|
@straithe any updates? |
|
I don't have the skillset to review this particularly well myself and the senior reviewers on my team are booked up for a couple months. I've asked in the GSG Discord for community reviews so I can ensure this solution works for a variety of systems. |
|
I tried building this, it was successful for windows, but the exe's didn't actually work for whatever reason. any thoughts @multiplemonomials ? |
|
What exactly didn't work? Did they fail to run? If so, that can be cause by having missing or incorrect DLLs in your system PATH. I recommend using Dependencies to load the executables and see what DLLs they are trying to load. |
|
@multiplemonomials you can download the artifacts here to try run it https://github.com/jLynx/hackrf/actions/runs/5151822083 And here is the workflow file https://github.com/jLynx/hackrf/actions/runs/5151822083/workflow And when I said it didn't run, i mean when I called the exe via cli, nothing was outputted, so errors, nothing. -h didn't return anything either |
|
Do the dll's get updated too, or are the the same ones from a few years ago? The reason I ask is because I copied some old ones in the dir to test and it didn't work. But I'm assuming they did get updated and that's the reason why @multiplemonomials |
|
Most likely, yeah. |
As far as I know, the same dependency DLL are used. These versions are the ones I use when building the latest host software on Windows and need to be included for libhackrf to operate: fftw-3.3.5-dll64 |
|
@multiplemonomials Is there is a reason that cmake version 3.8 is required for this? |
|
Yes, I believe that it does use some CMake 3.8 specific features. But 3.8 should be available on any modern linux distro at this point, it's a years old release. btw, want me to rebaae this MR? |
| install(TARGETS hackrf | ||
| RUNTIME DESTINATION bin | ||
| LIBRARY DESTINATION lib | ||
| ARCHIVE DESTINATION lib |
There was a problem hiding this comment.
Please include(GNUInstallDirs) and remove these DESTINATION (equivalent to using the CMAKE_INSTALL_* variables)
|
Will do, I can update this! Is this PR intended to be merged once I make the update? |
|
Now I find this issue... (not yet a pull request - especially since I see this issue I'm commenting in.) ci-scripts: host: Update project() call. Add some overall build defaults, Make use of CMake's GNUInstallDirs. Just use CMake's FindThreads. Rename FindUSB1 to FindLIBUSB, and have it provide the libhackrf: See CMake documentation for cmake-packages(7) hackrf-tools: Build with Modern CMake targets. Check to ensure project build with both Makefile and Ninja generators. TODO: check Windows build I haven't looked at multiplemonomials branch yet. 73 de aa4hs, |

Apologies for the issues that were encountered with the first version of my PR (#664), turns out that this build system was supposed to operate in ways I wasn't aware of. I went through all the feedback and made adjustments, and I believe I addressed all the issues with the original version. Summary of changes made:
host/,host/hackrf-tools/, andhost/libhackrf/To see the full set of changes vs the original PR, just look at all commits after the first commit in this PR.
To make sure things were working, I ran builds this PR on Windows MinGW, Windows MSVC, and Ubuntu. For each environment, I tried a build from each of the three supported top directories. I'm fairly confident that it should be working the way you want it now!