dev-libs/tvision: Version bump to 2.2.1.4 #6295
Conversation
|
Pull Request assignment Areas affected: ebuilds dev-libs/tvision: @gentoo/proxy-maint (maintainer needed) Bugs linked: 638794 |
gentoo-repo-qa-bot
added
maintainer-needed
|
-r4 in gentoo means a revision https://devmanual.gentoo.org/general-concepts/ebuild-revisions/ |
Moreover, 'revision bump' would not qualify as a good commit message. Just describe what you are doing. While we are at it, why do you add 'static'? |
|
@jonasstein I haven't read this chapter and was only comparing the naming of the current ebuild with the names of the source file and came to the wrong conclusion that the -r4 was because a -4 in the naming of the source code file. So what would you recommend in naming the ebuild? The source code file is named rhtvision-2.2.1-4.tar.gz. Just use tvision-2.2.1.ebuild and ignore the -4 from the source file? @a17r The commit message is not named 'revision bump', it's named 'Version bump' and that's what I see a lot when an ebuild gets it's version bumped. The current version is tvision-2.1.0_pre2-r4.ebuild. If you still have objections about the commit message, please let me know. I was adding the 'static' USE flag for several reasons:
Why do have objections to adding the static USE flag? In fact, I was wondering why it's not already there. A whole lot of libraries, at least such which are around for a long time, that offer both, static and shared libraries have a static USE flag. |
'Version bumps' are actual upstream bumps where that is of course the relevant news in git summary. Revisions are for ebuild changes and we don't announce those because they are implied. If upstream chooses tarball versions like
Is there any use for static-libs? If no one asked for it, just hard-disable the option. |
I like this 👍 just don't want to go the radical way in the first step. |
| S=${WORKDIR}/${PN} | ||
|
|
||
| # installed lib links to those | ||
| RDEPEND="sys-libs/ncurses:0 |
| x11-libs/libXau | ||
| x11-libs/libXdmcp | ||
| x11-libs/libSM | ||
| x11-libs/libICE ) |
There was a problem hiding this comment.
for multiline clauses, always do
X? (
foo/bar
other/pkg
)
|
|
||
| src_prepare() { | ||
| default | ||
| } |
There was a problem hiding this comment.
remove this, will happen anyways
| src_configure() { | ||
| ./configure --fhs \ | ||
| --cflags="${CFLAGS} --std=c99" \ | ||
| --cxxflags="${CXXFLAGS} --std=c++98" \ |
There was a problem hiding this comment.
does this really need C++98? Can't you rather fix it not to require C++98 crappiness
There was a problem hiding this comment.
Hmm... strangely I no longer get the errors I have been getting before, so I removed those. Thanks for the hint :)
Done
| dosym rhtvision /usr/include/tvision | ||
|
|
||
| # remove CVS directory which gets copied over | ||
| rm -rf "${D}/usr/share/doc/${PN}-${PVR}/html/CVS" |
There was a problem hiding this comment.
missing || die
also use ${ED%/} instead of ${D}
There was a problem hiding this comment.
Just a thought that came up to me: Wouldn't it even be better to remove the CVS directory inside the source tree before calling einstalldocs?
|
|
||
| src_install() { | ||
| emake DESTDIR="${D}" install \ | ||
| prefix="\${DESTDIR}/usr" \ |
There was a problem hiding this comment.
prefix="\${DESTDIR}/${EPREFIX}/usr" \
|
Build log for current status should be at https://drive.google.com/open?id=1xX_4Ykq2sBk4uIyOINkUKalxGufSXDRI |
waebbl
changed the title
|
|
||
| # installed lib links to those | ||
| RDEPEND="sys-libs/ncurses:0= | ||
| X? ( x11-libs/libX11 |
There was a problem hiding this comment.
the first line as well (the indentation looks weird otherwise)
There was a problem hiding this comment.
Done
I also changed the ${PN}-${PVR} in src_install rm command to ${P}, it's no longer necessary.
waebbl
force-pushed
the
tvision-bump
branch
2 times, most recently
from
b2e1570
to
0814223
Compare
| src_install() { | ||
| emake DESTDIR="${D}" install \ | ||
| prefix="\${DESTDIR}/${EPREFIX}/usr" \ | ||
| libdir="\$(prefix)/$(get_libdir)" || die |
| ) | ||
|
|
||
| src_configure() { | ||
| ./configure --fhs \ |
There was a problem hiding this comment.
Don't think so, it's no autoconf configure script, but an ancient perl based configure. It's copied over from the current ebuild, so I was guessing this doesn't work.
#!/bin/sh
perl config.pl "$@"
is the contents of the configure file.
There was a problem hiding this comment.
Perhaps you should leave comment about it.
There was a problem hiding this comment.
Good point! I'll do it
There was a problem hiding this comment.
Added comment about configure script, please review
waebbl
force-pushed
the
tvision-bump
branch
4 times, most recently
from
2a745d2
to
5ef7814
Compare
|
👍 All QA issues have been fixed! |
Copied over from release 2.1.0_pre2-r4 Add some USE flags: X, debug, gpm Update metadata.xml: Add maintainer, longdescription, use Application to proxy maintaining this package. Closes: https://bugs.gentoo.org/638794 Package-Manager: Portage-2.3.16, Repoman-2.3.6
Copied over from release 2.1.0_pre2-r4
Add some USE flags: X, debug, gpm, static
Update metadata.xml: Add maintainer, longdescription, use
Application to proxy maintaining this package.
Closes: https://bugs.gentoo.org/638794
Package-Manager: Portage-2.3.16, Repoman-2.3.6
Feedback welcome :)
Update: Renamed to tvision-2.2.1.4.ebuild, all patches for this version have also been renamed, include the review changes from @SoapGentoo