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

[OBS] Include excludelist in linuxdeployqt despite lack of access to the Internet #274

Closed
TheAssassin opened this issue Apr 9, 2018 · 8 comments

Comments

@TheAssassin
Copy link
Collaborator

CC @patrickelectric @t-paul @probonopd

I briefly mentioned this issue in #273. On the openSUSE build service (OBS), we don't have access to the Internet, which means we cannot simply generate the excludelist dynamically from the source file on GitHub.

We need to find a solution to get this information into the build. This solution:

  • should try to avoid adding maintenance overhead again (like, copying files from one repository to another, or having to update a Git submodule all the time)
  • must not require us to change source code within that huge single source file shared.c (if we have to violate the first point, we should rather add the data to an external header or some source file that can easily be changed without having to edit shared.c)
  • must work on OBS
  • should work on OBS without having to tell the OBS package maintainers to change the way linuxdeployqt is built at the moment (that is, having to change the call to qmake, etc.)

The last point would mean additional delay in providing a solution for end users.

In my opinion, it is OBS' duty to gather the "external resource" and pass it to qmake, but we don't even provide them with a way so they could implement this. Also I don't think they could implement this without writing something specific for linuxdeployqt. Using patches to hack the information into the build doesn't seem right either, because then, we would entirely lose control how often the excludelist is updated.

Therefore, I suggest a hybrid solution that only breaks with the first condition, but not entirely:

I would move the whole excludelist definition to a second source file called excludelist.cpp (with a header file excludelist.h).
We need to modify the script @patrickelectric wrote to generate such a source file on build to make it reproducible and reduce maintenance overhead, compared to the previous solution.
Then, we can occasionally commit this source file to linuxdeployqt's repository to keep it somewhat up to date.
The build system should, while configuring, or ideally on every build, always call this script and try to update the source file. If the script fails, there's an automatic "fallback" mechanism to use the old data. This should work on OBS just fine.
Whenever a maintainer builds on their normal workstation, the file will be updated, if there is access to the Internet. When they commit something next time to the repository, Git will ask them to commit the newly generated file, too, which would keep them relatively up to date. (Of course, they should be committed separately.)

This is a variety of the original solution (before #263) that mitigates the most annoying maintenance overhead, while providing a solution that works on OBS, and helps in keeping the file up to date. Also, our official builds will benefit from the automatic updates of the exclude list, only the builds on OBS and other no-Internet-access environments will be a bit behind the latest version.

What do you think?

@probonopd
Copy link
Owner

Sounds reasonable to me. Would be good if there were verbose messages during the build saying whether it has fetched data from the Internet or not.

@TheAssassin
Copy link
Collaborator Author

I am going to rewrite the shell script to produce a source file later, and will set up the CMake scripting. I don't know whether I will get to (or be willing to) make the required changes to qmake. Perhaps @patrickelectric can help with that.

TheAssassin added a commit that referenced this issue Apr 13, 2018
Improve excludelist generation (fixes #274)
@TheAssassin
Copy link
Collaborator Author

Fixed. Deadline has been adhered to. Satisfaction loading...

@t-paul
Copy link

t-paul commented Apr 15, 2018

I can confirm the packaging on OBS is back to working state (AppImage starting, no crash anymore).

@probonopd
Copy link
Owner

Fixed. Deadline has been adhered to. Satisfaction loading...

Thank you a thousand times 👍

@TheAssassin
Copy link
Collaborator Author

If it weren't a qmake based project, we could add some functionality tests. E.g., listing the installed libraries, and checking it against a set of validity tests. But I wouldn't want to develop something similar with qmake.

@probonopd
Copy link
Owner

qmake is like AppImage: a simple tool without too many bells and whistles that just does the job ;-)

But many in the Qt community use CMake for more complicated tasks. I remember a discussion in the Qt community about whether they should move to CMake altogether. Looks like they didn't.

@TheAssassin
Copy link
Collaborator Author

I disagree. qmake doesn't make anything easy. It's even more hackish than CMake. And really, CMake is a hack, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants