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

net-p2p/classified-ads: Install example files into correct path #22629

Closed

Conversation

operatornormal
Copy link
Contributor

Closes: https://bugs.gentoo.org/809464
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Antti Järvinen antti.jarvinen@katiska.org

Closes: https://bugs.gentoo.org/809464
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Antti Järvinen <antti.jarvinen@katiska.org>
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @operatornormal
Areas affected: ebuilds
Packages affected: net-p2p/classified-ads

net-p2p/classified-ads: @operatornormal, @gentoo/proxy-maint

Linked bugs

Bugs linked: 809464


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 self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Oct 18, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-18 20:20 UTC
Newest commit scanned: 48501bc
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/fb4530bad4/output.html

}

src_configure() {
eqmake5 examplefiles.path=/usr/share/doc/classified-ads-${PV}-r2/examples
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eqmake5 examplefiles.path=/usr/share/doc/classified-ads-${PV}-r2/examples
eqmake5 examplefiles.path=/usr/share/doc/${PF}/examples

@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE pkgmetadata SYSTEM "https://www.gentoo.org/dtd/metadata.dtd">
<!DOCTYPE pkgmetadata SYSTEM "http://www.gentoo.org/dtd/metadata.dtd">
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must admit I have no clue where this came from. I don't admit having touched metadata.xml. What I suspect is that I copied metadata.xml together with other files from /var/db/repos/.. into git tree, maybe it had http:// scheme in there. Anyway, this change is now reverted.

@@ -0,0 +1,45 @@
diff -u -r classified-ads-0.13.orig/classified-ads.pro classified-ads-0.13/classified-ads.pro
Copy link
Contributor

Choose a reason for hiding this comment

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

virtual/libintl"

DEPEND="${RDEPEND}
sys-devel/gettext
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is not in BDEPEND?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least gettext is not in @System set so to be safe it might be good idea to keep it. Or is there easy way to check what packages are included in BDEPEND?

Copy link
Contributor

Choose a reason for hiding this comment

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

BDEPEND has nothing to do with @System; think cross-compiling: if you're using arm machine to build the software for ppc64, do you need gettext for which arch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this took me some time to figure out. GNU gettext is build-time dependency (msgfmt program et al.) but the run-time functions required at runtime (setlocale+dgettext etc.) are found from /lib/libc.so.6 -> no extra run-time dep besides libc is needed. So I'll move gettext to BDEPEND.

Last time I was writing ebuild I had EAPI~6 or something and no BDEPEND. While studying documentation I came across a claim that contents of BDEPEND have something to do with system set and that left me confuzed. Bad internet.

I'll push corrected ebuild shortly..

Copy link
Contributor

Choose a reason for hiding this comment

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

Closes: https://bugs.gentoo.org/809464
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Antti Järvinen <antti.jarvinen@katiska.org>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-19 18:30 UTC
Newest commit scanned: 202fd64
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/56c90bd58b/output.html

Closes: https://bugs.gentoo.org/809464
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Antti Järvinen <antti.jarvinen@katiska.org>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-24 12:00 UTC
Newest commit scanned: ab51704
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/209a89713a/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.

sigh now this is a hard one. We definitely shouldn't patch and hardcode the ebuild revision version into the program, since you may get these massive tree-wide commits such as 8f2ed9e that require a revision bump, essentially breaking the program right?

I wonder if we/upstream can patch the program to just search for examples from e.g. /usr/share/classified-ads, and not under /usr/share/doc/ at all. Would that work better?

Also we like to install .html docs in /usr/share/doc/${PF}/html (minor issue)
https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-135003r4

Also if possible, can you rebase your branch against fresh master (the metadata.xml conflict) and squash your commits into a single one? Not stricly needed if you're not familiar with the process, but I had merge conflicts trying to test this.

@operatornormal
Copy link
Contributor Author

sigh now this is a hard one. We definitely shouldn't patch and hardcode the ebuild revision version into the program, since you may get these massive tree-wide commits such as 8f2ed9e that require a revision bump, essentially breaking the program right?

I wonder if we/upstream can patch the program to just search for examples from e.g. /usr/share/classified-ads, and not under /usr/share/doc/ at all. Would that work better?

Also we like to install .html docs in /usr/share/doc/${PF}/html (minor issue) https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-135003r4

Also if possible, can you rebase your branch against fresh master (the metadata.xml conflict) and squash your commits into a single one? Not stricly needed if you're not familiar with the process, but I had merge conflicts trying to test this.

This is a bit tricky yes ; this will work smoothly as long as upstream releases are released in ebuilds "as is" e.g. "2.0" will search for example files from "../doc/classified-ads-2.0/examples" but as soon as ebuild with version "-2.0-r1" or similar is needed, then a patch is needed also. Simple patch, but anyway.

One possibility is to have the program to search example files from any directory /usr/share/doc/classified-ads* .. or is there some magick mechanism to have multiple versions at the same time? There could be incompatible examples files between different versions. /usr/bin/classified-ads can exists only once, I'm wondering why there needs to be multitude of document directories when it is possible to have the documented binary from one version only?

I'd also go with the idea that example files are moved away from /usr/share/doc to something more datadir kind of folder and naming of that directory could be static.

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.

Let's fix it for now, but you'll probably get a new report if/when revision is bumped again. You know what to do as upstream ;)

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. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
6 participants