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

QT doesn't need svg #195

Merged
merged 1 commit into from Aug 27, 2014
Merged

QT doesn't need svg #195

merged 1 commit into from Aug 27, 2014

Conversation

ZeroChaos-
Copy link
Contributor

It looks like QT doesn't need svg here. This causes the binary to attempt linking to qtsvg, however, since gqrx doesn't actually link to qtsvg, this seems kinda silly.

the downstream bug is here:
https://bugs.gentoo.org/show_bug.cgi?id=521206

This PR removes the build time dep (since it isn't used at build time). QTsvg should be a runtime dep only.

It looks like QT doesn't need svg here.  This causes the binary to attempt linking to qtsvg, however, since gqrx doesn't actually use qtsvg, this seems kinda silly.

the downstream bug is here:
https://bugs.gentoo.org/show_bug.cgi?id=521206

Are we missing something?
@csete
Copy link
Collaborator

csete commented Aug 27, 2014

Are you sure we don't need that? All the icons are in SVG format so at some point the SVG API is needed. I can't see the bug at the moment but will check it later.

@ZeroChaos-
Copy link
Contributor Author

I am 100% certain that it doesn't need to be linked to qtsvg, as the linking actually is a no-op and the final binary is not linked to qtsvg when you use --as-needed (gentoo default).

This patch prevents qtsvg from being on the linker line for no reason at all.

QTsvg is actually needed as a runtime dep as far as I can tell, which means it needs to be installed to run gqrx, but not to build.

@csete
Copy link
Collaborator

csete commented Aug 27, 2014

Ok, thanks for the clarification.

csete added a commit that referenced this pull request Aug 27, 2014
@csete csete merged commit f186e68 into gqrx-sdr:master Aug 27, 2014
@ZeroChaos-
Copy link
Contributor Author

I didn't include it in my PR, but you may wish to add a note that QTsvg is needed at runtime?

@ZeroChaos-
Copy link
Contributor Author

Most distros split up QT quite a bit, in gentoo for instance we have qtgui and qtcore as build deps, and qtsvg as a runtime dep. I can amend the README.md at your request, or you can handle it as you see fit.

@csete
Copy link
Collaborator

csete commented Aug 27, 2014

Yeah, most distributions have separate packages for the different components, then both Qt 4 and 5 :)
I have updated the readme to list the required Qt components.

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

Successfully merging this pull request may close these issues.

None yet

2 participants