-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
New package: gammaray-2.11.1 #23242
New package: gammaray-2.11.1 #23242
Conversation
hm I don't know what changed. I installed the software on my pc and it built fine last time. |
Added this patch KDAB/GammaRay@7cd17bc. Should work now |
srcpkgs/gammaray/template
Outdated
distfiles="https://github.com/KDAB/GammaRay/releases/download/v${version}/${pkgname}-${version}.tar.gz" | ||
checksum=87a1d72ad1ad6d1a0156c54a85b0976ab38c6a64136458ca7c4ee491566d25d0 | ||
|
||
LDFLAGS+="-Wl,-no-fatal-warnings" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want to put a space there " -Wl..."
srcpkgs/gammaray/template
Outdated
LDFLAGS+="-Wl,-no-fatal-warnings" | ||
|
||
pre_build() { | ||
sed -e 's|plugins/gammaray|lib/qt5/plugins/gammaray|' -i CMakeLists.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, use the vsed wrapper.
srcpkgs/gammaray/template
Outdated
maintainer="toluschr <toluschr@protonmail.com>" | ||
license="GPL-2.0-or-later" | ||
homepage="https://github.com/KDAB/GammaRay" | ||
distfiles="https://github.com/KDAB/GammaRay/releases/download/v${version}/${pkgname}-${version}.tar.gz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"${homepage}/..."
srcpkgs/gammaray/template
Outdated
cd "${DESTDIR}" | ||
for f in usr/lib/*.so; do [ -L "${f}" ] && vmove "${f}"; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it ship any .so
files that aren't symlinks? Ideally this line should be vmove "usr/lib/*.so"
.
And I'm not entirely sure that cd
makes everything work normally, you should probably check the package contents afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I'm not entirely sure that
cd
makes everything work normally
I used cd to make the glob expression use the relative path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like vmove
is smart enough to deal with it. Are there too many of these files, and some specific reason for them not being versioned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why the shared libraries are not versioned. I believe they act as plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. If you could leave a comment there, then. Like "unversioned dynamic libraries should go in main package"
@@ -0,0 +1,31 @@ | |||
# Template file for 'gammaray' | |||
pkgname=gammaray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package name should probably be GammaRay. I believe repo name takes precedence over tarball.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, didn't see that earlier. IMHO the package name should be gammaray
since that's the name of both the program binary and the source tarball.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure either. The name in its desktop file is GammaRay though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use gammaray
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preference as a user would be "least surprise". The executable is called gammaray
, so the package should be called that too. The user doesn't know about repos, and the application name in the desktop file is a (possibly localized) title for presentation purposes only.
26c7b27
to
fc94546
Compare
srcpkgs/GammaRay/template
Outdated
vmove usr/lib/cmake | ||
vmove usr/lib/qt5/mkspecs | ||
# probes should go in main package | ||
vmove "usr/lib/libgammaray_[^probe]*.so" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The libraries are conveniently named. This should be much better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ is not a negative match over string. Please check if results are as expected. You may need to vmove all libgammaray them move back probe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure that it is correct either, but it results in the correct libraries being moved. Perhaps moving the libraries back would be the best solution
srcpkgs/GammaRay/template
Outdated
vmove usr/lib/qt5/mkspecs | ||
cd "${DESTDIR}" | ||
# unversioned probes should go in main package | ||
for f in usr/lib/*.so; do [ -L "${f}" ] && { vmove "${f}"; }; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep it like that for now. It seems to be the solution using the least characters. I hope this is ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer gammaray
:p
srcpkgs/GammaRay/template
Outdated
@@ -0,0 +1,34 @@ | |||
# Template file for 'GammaRay' | |||
pkgname=GammaRay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The app binary is called gammaray
, as is the source tarball, so the package should also be called gammaray
IMO.
srcpkgs/GammaRay/template
Outdated
|
||
LDFLAGS+=" -Wl,-no-fatal-warnings" | ||
|
||
pre_build() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be pre_configure
or I'd expect the vsed
substitution below to accumulate in incremental builds, i.e. if a xbps-src pkg
terminates during the build
phase, on the next run the result will probably be lib/qt5/lib/qt5/plugins/gammaray
, etc.
In clean builds, this would save the second cmake
run when it detects that the files have changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion.
But configure
sometimes fails, too. Let's say in next version, they add new dependencies.
For that reason, I prefer to use post_patch
instead.
It matches with the action: we're patching the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, of course, that's even better!
common/shlibs
Outdated
libgammaray_core-qt5_15-x86_64.so.2.11.1 GammaRay-2.11.1_1 | ||
libgammaray_common-qt5_15-x86_64.so.2.11.1 GammaRay-2.11.1_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -qt5_15-x86_64
part should be removed or shlibs
will be off on other architectures. To make the build produce sane file names, CMakeLists.txt
will have to be fixed, like in the shiboken2
package (patch).
srcpkgs/GammaRay/template
Outdated
distfiles="${homepage}/releases/download/v${version}/gammaray-${version}.tar.gz" | ||
checksum=87a1d72ad1ad6d1a0156c54a85b0976ab38c6a64136458ca7c4ee491566d25d0 | ||
|
||
LDFLAGS+=" -Wl,-no-fatal-warnings" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first occurence of LDFLAGS
doesn't need +=
LDFLAGS="-Wl,-no-fatal-warnings"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it added to the environment's FLAGS, sorry! Which I now realized doesn't make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template will be sourced first. The various hooks and setup will prepend/append necessary arguments into various flags, including but not limiting to configure_args
LDFLAGS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks.
f90c844
to
2ac3f3a
Compare
Any issues with the newest commit? |
6e075ca
to
31db75e
Compare
Just wanted to test this, and it doesn't seem to work at all. |
It seems that gammaray tries to load the wrong probe library. The patch that removes the |
@Johnnynator
|
#22756