-
Notifications
You must be signed in to change notification settings - Fork 429
Qt module project structure #124
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
Conversation
This PR is blocked by #127. Please wait for that PR to be merged, then you'll have to update this one. Looks like it's anyway conflicting. |
PR #127 has been merged, so you can update your PR now. I'd suggest to replace qmake by CMake completely completely to improve the integration with other tools. E.g., we're intending to replace AppImageUpdate's and zsync-curl's build system with it, and it would be nice to have the same tool used across all related projects. But as I am aware of #84, and thus won't try to establish CMake. Also, please don't suggest to build in-source (like you did with your example), that's a general rule of thumb. It leads to a vast amount of problems in my experience, you should generally avoid it. I know that the Travis config does that at the moment, but we're going to change that soon anyway. To simplify the structure, I'd just put all sources and headers in the same directory. We don't really have to split it up. But moving them to the same directory is a first step. |
I'll wait until #125 is merged, then I will check how much of this PR is still relevant, and update it accordingly |
it was only shared for mac, becaused it was used by 2 tools
I updated to the latest master. Since #125 seems to be stale, I made this PR ready to merge. But before you do, a few words to explain the PR:
And one more, rather important thing:
Edit: The codacy warnings are not mine, they come from the master branch |
Oh, and @TheAssassin regarding the build in source - it was just for simplicity. I do not build in source myself. (But as far as I know, The Qt module setup tries to prevent such problems through it's setup) |
Thanks @Skycoder42 |
Ah, one more thing: You might already have seen it in travis, but to build both the tool and all tests, one can simply run |
To this very day I do not understand what the advantage of this "Qt module project structure" actually is. Apparently it just makes stuff more complicated compared to what we had before. @Skycoder42 can you please explain the motivation behind this PR?
Do we really want that? I mean, we are producing an AppImage of |
This seems to be causing
Reference: |
The idea behind this was that if you ever contribute this to Qt, you already have the project structure that is required for that. If you check the different pro files, you will see a bunch of The warning means that the As long as you're not planning to actually contribute, there is not much of an advantage. I personally am using linuxdeployqt for deployment only, and not to generate an AppImage, just like I use windeployqt and macdeployqt. In that role, it fits pretty well as "Qt internal tool" that is compiled with Qt. And having it as part of the Qt installations makes shure it always uses the correct qmake etc. |
Thanks for the clarification @Skycoder42. Can you tell me how to build linuxdeployqt using the Qt from qt.io? Because that is what I am trying to do. |
That should work. I am using it myself in a build where I build the continous linuxdeployqt release (See https://travis-ci.org/Skycoder42/qpmx/jobs/301567325#L1381) and I don't get that error and the build succeeds. Maybe try to explicitly invoke the qmake installed? or add EDIT: I use |
This pull request aims to refactor the project structure to make it build as a "Qt module". It's a first step to get this tool closer to Qt.
The main advantage here is: Simply running:
will compile and install the tool into your Qt installation, and make it a part of your Qt just like any other tool (qmake, etc.)