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
app-misc/mosquitto: bump version to 1.4.15 #7362
Conversation
Pull Request assignment Areas affected: ebuilds app-misc/mosquitto: @lramage94, @gentoo/proxy-maint No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and ping us to reset the assignment. In order to force reassignment and/or bug reference scan, please append |
DESCRIPTION="An Open Source MQTT v3 Broker" | ||
HOMEPAGE="http://mosquitto.org/" | ||
SRC_URI="http://mosquitto.org/files/source/${P}.tar.gz" | ||
LICENSE="EPL-1.0" |
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.
Empty line above it. What is the source of this bad template a lot of people seem to be using?
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 just copied the old ebuild and renamed it via mv
. Where should I put the new line?
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.
Between SRC_URI
and LICENSE
.
} | ||
|
||
src_configure() { | ||
LIBDIR=$(get_libdir) |
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.
Why are you declaring it as a global variable?
|
||
src_configure() { | ||
LIBDIR=$(get_libdir) | ||
makeopts=( |
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.
It's up to you but I'd say it's generally cleaner to add a wrapper function that adds all the args than to keep them in global variable.
} | ||
|
||
src_install() { | ||
emake "${makeopts[@]}" DESTDIR="${D}" prefix=/usr install |
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.
${EPREFIX}/usr
maybe?
if use examples; then | ||
docompress -x "/usr/share/doc/${PF}/examples" | ||
insinto "/usr/share/doc/${PF}/examples" | ||
doins -r examples/* |
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.
dodoc
/docinto
, also examples/.
to avoid unnecessary glob.
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 are supposed to test your ebuilds before pushing the changes.
srv? ( net-dns/c-ares ) | ||
websockets? ( net-libs/libwebsockets )" | ||
|
||
makeopts() { |
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.
But this just doesn't do anything?
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.
what is the proper method?
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.
Make the function do emake ... "${@}"
, and call it instead of emake
.
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.
Can you show me an example of this? I am looking in the tree and I see other packages that do it the other way. For example, net-misc/dropbear-2018.76 has a set_options function that has a makeopts=(...) in it.
src_prepare() { | ||
eapply "${FILESDIR}/${PN}-1.4.10-conditional-tests.patch" | ||
if use persistence; then | ||
sed -i -e "s:^#autosave_interval:autosave_interval:" \ |
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.
/autosave_interval/s:^#::
and like that?
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.
What do you mean?
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 mean to simplify the sed expression. man sed
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 persistence is enabled, save the in-memory database to disk
# every autosave_interval seconds. If set to 0, the persistence
# database will only be written when mosquitto exits. See also
# autosave_on_changes.
# Note that writing of the persistence database can be forced by
# sending mosquitto a SIGUSR1 signal.
#autosave_interval 1800
# If true, mosquitto will count the number of subscription changes, retained
# messages received and queued messages and if the total exceeds
# autosave_interval then the in-memory database will be saved to disk.
# If false, mosquitto will save the in-memory database to disk by treating
# autosave_interval as a time in seconds.
#autosave_on_changes false
# Save persistent message data to disk (true/false).
# This saves information about all messages, including
# subscriptions, currently in-flight messages and retained
# messages.
# retained_persistence is a synonym for this option.
persistence true
# The filename to use for the persistent database, not including
# the path.
#persistence_file mosquitto.db
# Location for persistent database. Must include trailing /
# Default is an empty string (current directory).
# Set to e.g. /var/lib/mosquitto/ if running as a proper service on Linux or
# similar.
#persistence_location
insinto /etc/mosquitto | ||
doins mosquitto.conf | ||
|
||
if use systemd; then |
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.
This is invalid use of USE=systemd.
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.
How so? That is how examples
is being used. Where are the rules against this?
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.
$ quse -D systemd
global:systemd: Enable use of systemd-specific libraries and features like socket activation or session tracking
https://wiki.gentoo.org/wiki/Project:Systemd/Ebuild_policy#General_guidelines
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 note that unit files are still installed unconditionally to the USE flag."
I see this now, thank you.
|
||
if use examples; then | ||
docompress -x "/usr/share/doc/${PF}/examples" | ||
docinto "/usr/share/doc/${PF}/examples" |
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.
This is wrong.
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.
What is the correct solution?
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.
Read what docinto
does and you'll know. You're expected to show some initiative, not wait for developers to give you complete copy'n'paste solutions.
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 have fixed all of the previous issues. But if the qa tools are not giving me any errors, and if the package builds and runs sucessfully then I fail to see the reason behind some of these changes. It was working fine when it was globbing.
Well, I wanted to show you the problem but the ebuild doesn't even get there:
|
} | ||
|
||
src_test() { | ||
_emake test |
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.
IUSE="$IUSE test" # append test to IUSE
REQUIRED_USE="test? ( bridge )" # this is needed for the tests to pass
Is this appropriate?
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.
Presuming this is an explanation of what you're going to do and not literal snippet, yes.
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, thanks.
Package-Manager: Portage-2.3.19, Repoman-2.3.6
Looks good, thanks. I'm going to merge it. |
Package-Manager: Portage-2.3.19, Repoman-2.3.6