-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update MacOS and Windows build instructions and remove old build scripts #69
Conversation
please drop everything related to building Qt from source for Windows you can also reference alternative way to obtain Qt: msys2 packages, see usb-sd-creator/.github/workflows/build.yaml Lines 61 to 65 in bb64dfd
|
Will this just happen automatically now with the new build system? I.e. QT will get downloaded as part of the cmake build? if not, what should be the instructions to installl the required QT libs? |
Qt isn't downloaded automatically, of course. You should install it in any way you wish: Qt online installer, Homebrew, msys2, your package manager on Linux, Conan, vcpkg, build from source etc. Giving details on this process normally is out of scope of a project. |
54098a4
to
b76489e
Compare
I'll add a separate commit for windows, but I'll need someone to test it. Let's use |
7ea1613
to
3f78abc
Compare
aa50f2d
to
c5aaa2d
Compare
c5aaa2d
to
7c3542e
Compare
Then I suggest to try on an older version where it's supposed to work. Could be some Sonoma change for example.
Because this is not your folder, it's of the root user. |
The earlier version I have doesn't run on ARM and I have no Intel HW these days so I'm unable to test. However, when we first started reworking things I was using Intel HW (on whatever macOS was current then) and the difference was before/after we started reworking the code. I'm confident it's not related to the macOS version.
That makes complete sense, but it's seriously confusing for users. I guess this has existed since forever, but I never noticed the issue in the past because I was in the habit of using drag/drop to set the path. |
Intel binaries should run just fine on ARM.
|
Ok, so I tied to build on windows (after finally get a windows PC working) and cmake is failing on finding zlib. I would have thought they comes with QT. I used AQT and installed the 4 windows archives from the github action. Did I miss something? |
Hmmm, so I installed gnu zlib but then it attempts to open unistd.h that doesn’t exist on windows. |
Ok, that was with msvc, so I moved to mingw via msys and that worked. Still had to install zlib via msys and provide the paths for cmake, but it worked and tested correctly. Will add instructions shortly. |
0a09f31
to
e948e30
Compare
repo contains prebuilt zlib only for msvc 2019: Lines 43 to 45 in bb64dfd
for other msvc toolsets one should build it manually, see https://github.com/madler/zlib/blob/develop/win32/VisualC.txt / https://github.com/madler/zlib/tree/develop/contrib/vstudio |
Qt contains only static zlib that is used inside Qt itself |
e948e30
to
9708436
Compare
correct spelling is Qt, not QT (the latter traditionally means QuickTime) |
Ok, msvc 2022 is the community edition, maybe I'll build that and add it to the repo. What about on mingw, I didn't think I'd need to specify the ZLIB paths but I had to. |
So the release build works via mingw. But when using msvc it does not, as it tried to use the x86 compiler and not 64 bit. Any idea why msvc only has this problem with release preset, but debug builds ok?
|
I got it to work on msvc using:
However, then creating the installer fails using:
|
Aha, it's building the Release build into Debug for some reason. If I rename the directory the installer builds correctly. No idea why this is happening. |
So if I run any msvc build the same error always occurs: doesn’t matter if it’s a Debug or release build. Same error every time. I already added your patch for the JSON parser @kambala-decapitator |
5171ab0
to
f3052a8
Compare
Current state:
The error in #69 (comment), present n Debug builds in words:
|
f3052a8
to
e4a2949
Compare
Added instructions to start an x64 developer command prompt when building with msvc. So we now get a release build for msvc. Updated current state to reflect this. |
e4a2949
to
b85c544
Compare
I tested the CI build for msvc and it installs and works most of the time. There is still as issue where it can hang on writing to SD, similar to my local MingW build. So the QT error I get when building MSVC locally must be to do with my env, not sure exactly what it is. @kambala-decapitator I wanted to do a similar test for the mingw build, but pack is not being run in CI for MinGW, any idea what that is? Maybe it does not detect cpack??? |
cf5e4c6
to
b85c544
Compare
@phunkyfish your crash screenshot indicates that Qt debug version was picked, using it with app release build is incorrect (at least when building with msvc). To know the reason of the crash, run app under debugger from visual studio.
sounds like something Windows-specific, no idea where to look
because we aren't going to distribute mingw build to end users, so I never cared to test cpack for it. For local development running cpack isn't needed. |
Ok, this means that CI only uploads a ZIP artifact and not an installer. It would be easier to have both for testing purposes from CI. |
currently mingw platform is tested only as a build target (sanity check), CI doesn't pack artifacts for it |
Ok, but the hanging on write sometimes is happening cross platform. Both Mac (Intel and Apple silicon), my local mingw build and the CI build of msvc. I’ll put one of them on a debugger and see if I can figure out where exactly the hang occurs. |
please also check if it occurs in the old macOS build: https://chrishewitt.net/drive/d/s/wb9vTYFxHZOM6Jcyik5zP84MX4ErxALi/1pJlW9XyAAu50Xml31KixTIRAWcJGlJU-WLLgcoUSkAs |
Will do. I'll look this evening. I assume the old mac build is intel only? |
Here is a gist of the msvc crash from the VS debugger: https://gist.github.com/phunkyfish/3549999cd329e751ec5a8ef099cf488c @kambala-decapitator screenshot of debugger window and call stack: Note that I have your JSON improvements cherry-picked on the build: |
I guess so. But it should run on ARM anyway.
the error on your previous SS says about out of bounds access, but there's nothing like that here. However, now I see that there's no check for iterator validness like it's done for |
let me add it and I’ll see if it makes a difference. |
b85c544
to
afb0738
Compare
afb0738
to
82f35c7
Compare
Ok, that worked. MSVC now builds and installs and runs correctly. |
Ok, I could not reproduce the hang on write (on intel or apple silicon) with the old build from @chewitt. I reckon we merge these PRs now anyway. I can attach a debugger to the mac build locally and figure out that issue separately but will wait until both PRs are merged first. |
Please review and merge once you are happy with this @chewitt |
Migrate build instructions to new cmake build system. Include updating the README, adding dependencies and removing obsolete build files.