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

sys-fs/compsize: New ebuild (proxy maintainer) #7660

Closed
wants to merge 1 commit into from

Conversation

automorphism88
Copy link
Contributor

I wrote this ebuild and went to open a bug on Bugzilla so I could submit a PR for it and noticed that bug #646616 already existed, so I didn't open a duplicate. However, this ebuild is my own work, tested by me, and I am willing to proxy-maintain it.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: sys-fs/compsize

sys-fs/compsize: @gentoo/proxy-maint (new package)

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 [please reassign] to the pull request title.

@gentoo-repo-qa-bot gentoo-repo-qa-bot added new package The PR is adding a new package. 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). labels Mar 27, 2018
# Copyright 1999-2018 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

EAPI=6
Copy link
Member

Choose a reason for hiding this comment

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

Please use the standard order as present in skel:

EAPI
inherit ....

DESCRIPTION
HOMEPAGE
SRC_URI

LICENSE
SLOT
KEYWORDS
IUSE

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 include the appropriate empty lines too.

RDEPEND=""
inherit flag-o-matic
# Used in upstream Makefile, but clobbered by portage's CFLAGS
append-cflags -Wall -std=gnu90
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 do this in global scope, move it to a phase function.

# Used in upstream Makefile, but clobbered by portage's CFLAGS
append-cflags -Wall -std=gnu90

if [[ "$PV" = 9999 ]] ; then
Copy link
Member

Choose a reason for hiding this comment

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

Always use ${...} variable form. Also, don't quote inside [[ ... ]].


src_install() {
# work around Makefile failing to create this directory
mkdir -p "${ED}/usr/share/man/man8" || die
Copy link
Member

Choose a reason for hiding this comment

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

${ED%/}

<maintainer type="project">
<email>proxy-maint@gentoo.org</email>
</maintainer>
<longdescription>utility to measure btrfs compression ratio</longdescription>
Copy link
Member

Choose a reason for hiding this comment

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

Remove this, please. It's only necessary when you have a very long description that doesn't fit in ebuild.

<email>adebeus@gmail.com</email>
</maintainer>
<maintainer type="project">
<email>proxy-maint@gentoo.org</email>
Copy link
Member

Choose a reason for hiding this comment

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

Please include <name/> here (copy it from some other ebuild).

@automorphism88
Copy link
Contributor Author

The new commit should fix all of these issues.

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Once done, please squash all you commits into one.


if [[ ${PV} = 9999 ]] ; then
inherit git-r3
KEYWORDS=""
Copy link
Member

Choose a reason for hiding this comment

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

Remove the two empty vars. Otherwise ekeyword used by arch testers will add keywords here.

# Copyright 1999-2018 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

EAPI=6
Copy link
Member

Choose a reason for hiding this comment

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

Please include the appropriate empty lines too.

@@ -0,0 +1,30 @@
# Copyright 1999-2018 Gentoo Foundation
Copy link
Member

Choose a reason for hiding this comment

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

Update -9999 as well.

@automorphism88 automorphism88 force-pushed the compsize branch 2 times, most recently from 060dcc2 to 7582d10 Compare March 30, 2018 20:26
@automorphism88
Copy link
Contributor Author

How does it look now?

@mgorny
Copy link
Member

mgorny commented Mar 30, 2018

Almost good. One more thing to fix:

>>> /usr/share/man/man8/compsize.8.gz.lz

(my Portage might be stricter than yours)

Please fix the build system not to compress manpages on its own.

@automorphism88
Copy link
Contributor Author

I actually had put that fix in originally, but replaced it with the mkdir -p in src_install() upon finding that my portage seemed to uncompress the man page before recompressing it, in the interest of keeping the upstream Makefile intact. I've reverted that change, so now the line in the Makefile that gzips the man page into the directory created with mkdir -p is removed with sed, and the man page is manually installed with doman.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-03-30 21:28 UTC
Newest commit scanned: da15719
Status: ✅ good

Issues already there before the PR (double-check them):
https://qa-reports.gentoo.org/output/gentoo-ci/5c2cb7f0b/output.html#net-fs/samba

@mgorny
Copy link
Member

mgorny commented Mar 30, 2018

Could you try passing MAN_I= (empty) to emake install? That would probably be a little cleaner than sed ;-).

@automorphism88
Copy link
Contributor Author

I just tried that, but it doesn't seem to work.

@automorphism88
Copy link
Contributor Author

In particular, after applying this change:

diff --git a/sys-fs/compsize/compsize-1.1.ebuild b/sys-fs/compsize/compsize-1.1.ebuild
index ad73046..6437dc2 100644
--- a/sys-fs/compsize/compsize-1.1.ebuild
+++ b/sys-fs/compsize/compsize-1.1.ebuild
@@ -21,13 +21,6 @@ SLOT=0
 RDEPEND=""
 DEPEND="sys-fs/btrfs-progs"
 
-src_prepare() {
-	eapply_user
-	# Don't try to install a gzipped manfile during make install, instead
-	# use doman in src_install to ensure that PORTAGE_COMPRESS is used
-	sed -i $'/^\tgzip /d' Makefile || die
-}
-
 src_configure() {
 	# Used in upstream Makefile, but clobbered by portage's CFLAGS
 	append-cflags -Wall -std=gnu90
@@ -35,7 +28,7 @@ src_configure() {
 }
 
 src_install() {
-	emake PREFIX="${ED%/}" install
+	emake MAN_I= PREFIX="${ED%/}" install
 	doman compsize.8
 	einstalldocs
 }

the installation fails with the following:

>>> Install compsize-1.1 into /var/tmp/portage/sys-fs/compsize-1.1/image/ category sys-fs
make -j5 MAN_I= PREFIX=/var/tmp/portage/sys-fs/compsize-1.1/image install 
install -Dm755 compsize /var/tmp/portage/sys-fs/compsize-1.1/image/usr/bin/compsize
gzip -9n <compsize.8 >/var/tmp/portage/sys-fs/compsize-1.1/image/usr/share/man/man8/compsize.8.gz
/bin/sh: 1: cannot create /var/tmp/portage/sys-fs/compsize-1.1/image/usr/share/man/man8/compsize.8.gz: Directory nonexistent

So I'm leaving it as-is for now unless you can suggest a better way to fix it.

@mgorny
Copy link
Member

mgorny commented Mar 30, 2018

Ok, thanks for testing. Let's go with sed then.

@automorphism88 automorphism88 deleted the compsize branch March 30, 2018 22:18
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). new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
3 participants