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

media-sound/mpdscribble: add version 0.23 #22254

Closed
wants to merge 2 commits into from

Conversation

ArsenArsen
Copy link
Member

@ArsenArsen ArsenArsen commented Sep 9, 2021

Addresses https://bugs.gentoo.org/812275 and https://bugs.gentoo.org/784398

Keywords in 0.23 were adjusted to my testing abilities (and even with that might be inaccurate still, since I haven't explicitly tested on x86 alone). I believe bug https://bugs.gentoo.org/605374 can also be addressed in this commit, with some further changes.

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Sep 9, 2021
@ArsenArsen ArsenArsen changed the title media-sound/mpdscribble: add version 0.23 media-sound/mpdscribble: add version 0.23 [please reassign] Sep 9, 2021
@gentoo-bot gentoo-bot changed the title media-sound/mpdscribble: add version 0.23 [please reassign] media-sound/mpdscribble: add version 0.23 Sep 9, 2021
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @ArsenArsen
Areas affected: ebuilds
Packages affected: media-sound/mpdscribble

media-sound/mpdscribble: @gentoo/sound

Linked bugs

Bugs linked: 812275, 784398


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. and removed assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Sep 9, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-09-09 12:40 UTC
Newest commit scanned: c5b813c
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/b80787c9b2/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-09-09 12:55 UTC
Newest commit scanned: 12c9560
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c821b64b45/output.html

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay let's process this. Few notes.

# Copyright 1999-2021 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

EAPI=7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 should work now.

inherit meson systemd

DESCRIPTION="An MPD client that submits information to Audioscrobbler"
HOMEPAGE="https://www.musicpd.org/clients/mpdscribble/"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add the Github page as 2ndary HOMEPAGE here?

Comment on lines 17 to 24
media-libs/libmpdclient
net-misc/curl
dev-libs/boost
dev-libs/libgcrypt
"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sort

Comment on lines 1 to 29
From 0921e6816d390d219f4e7a4fe72d10a17d67e359 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <arsen@aarsen.me>
Date: Thu, 9 Sep 2021 14:19:01 +0200
Subject: [PATCH] Don't install AUTHORS, COPYING, NEWS, README.rst

We install these in ebuilds.
---
meson.build | 5 -----
1 file changed, 5 deletions(-)

diff --git a/meson.build b/meson.build
index b4cc736..28ade21 100644
--- a/meson.build
+++ b/meson.build
@@ -128,11 +128,6 @@ executable(
install: true
)

-install_data(
- 'AUTHORS', 'COPYING', 'NEWS', 'README.rst',
- install_dir: join_paths(get_option('datadir'), 'doc', meson.project_name()),
-)
-
subdir('systemd')

subdir('doc')
--
2.32.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there's some sort of conflict with compressing these files properly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, a version with and without compression would be installed


DOCS=( AUTHORS COPYING NEWS README.rst )

DEPEND="${RDEPEND}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this below RDEPEND.


src_prepare() {
# we install these below via systemd_do{,user}unit
sed -i '/install_dir/d' -i systemd/*/meson.build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you already introduce some sort of patch file for this? Why's the sed needed anymore? Besides, external commands (sed) needs to || die.

media-libs/libmpdclient
net-misc/curl
dev-libs/boost
dev-libs/libgcrypt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if this gets linked, add a subslot binder,
dev-libs/libgcrypt:=

This version of mpdscribble uses meson to build, so a full rewrite of
the ebuild was needed.

Closes: https://bugs.gentoo.org/812275
Package-Manager: Portage-3.0.20, Repoman-3.0.3
Signed-off-by: Arsen Arsenović <arsen@aarsen.me>
This ebuild is broken due to upstream changes.

Closes: https://bugs.gentoo.org/784398
Package-Manager: Portage-3.0.20, Repoman-3.0.3
Signed-off-by: Arsen Arsenović <arsen@aarsen.me>
@ArsenArsen
Copy link
Member Author

All that should be fixed, I believe.

BTW, is the sort order for fields defined anywhere? For future reference

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-09-27 09:26 UTC
Newest commit scanned: 2a834dc
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6dc34cf86c/output.html

@juippis
Copy link
Member

juippis commented Sep 28, 2021

BTW, is the sort order for fields defined anywhere? For future reference

Theory:
https://devmanual.gentoo.org/general-concepts/dependencies/index.html

Practical example:
https://wiki.gentoo.org/wiki/Project:Proxy_Maintainers/User_Guide/Style_Guide#Sorting_dependencies

EDIT: if you mean the variables, we try to follow the ebuild.skel for tree-wide consistency:
https://github.com/gentoo/gentoo/blob/master/skel.ebuild

FYI getting:

 * QA Notice: This ebuild installs into paths that should be created at runtime.
 *  To fix, simply do not install into these directories.  Instead, your package
 *  should create dirs on the fly at runtime as needed via init scripts/etc...
 * 
 *   var/cache
 *   var/cache/mpdscribble

Contribution looks fine, but I'd like to hear your opinion on this: Do you think you can modify the systemd service file & openrc init script to accomplish this? It's not a requirement, will merge if you don't want to put time for that now, but it'd be a great extra. Let me know.

@ArsenArsen
Copy link
Member Author

I did ask about this on IRC, since this is an old issue. This seems like a job for the *tmpfile implementations. I can add the configuration for those.

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well let's merge and fix the issues this PR was originally raised for. If you do end up fixing the init.d/service files, please ping me directly in Github so I see it (not part of the project that maintains this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. hacktoberfest-accepted PR accepted for Hacktoberfest
Projects
None yet
4 participants