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/prog-express: New package #7602
Conversation
doins "${FILESDIR}"/pe.exe.config | ||
|
||
# Install udev rule | ||
insinto "/lib/udev/rules.d" |
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 use udev.eclass
for 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.
Thanks, I will update the eBuild to use the udev class.
done | ||
|
||
# Install bin files | ||
dobin "usr/bin/bxusb" "usr/bin/bxusb-gui" "usr/bin/prog-express" |
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.
Minor: you do not need all double quotes for ebuild helpers here. I mean insinto
dosbin
doins
domenu, doman
etc
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.
Okay, I will revert those double quotes, personally, I like them.
|
||
EAPI=6 | ||
|
||
inherit eutils multilib |
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.
Any explanation why are you using multilib.eclass
here? I guess you did this to provide $(get_libdir)
feature, but this is a built-in function for EAPI=6
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 right, I wasn't aware, that this is part if EAPI=6. Will be fixed.
|
||
EAPI=6 | ||
|
||
inherit eutils multilib |
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 do not see any reason to use eutils.eclass
here as well, please use desktop.eclass
, it provides functionality to handle menus and icons :)
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.
Ah, I see, will be changed.
|
||
QA_PREBUILT="usr/bin/bxusb usr/bin/bxusb-gui usr/bin/prog-express usr/sbin/bxfxload" | ||
|
||
src_unpack() { |
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 do not handle this manually, use unpacker.eclass
for this purpose
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.
Thanks, I will update the eBuild.
@Zlogene: Thank you very much for you review, I've pushed a new version, which adds all of your requested changes. |
|
||
src_install() { | ||
# Fix lib path | ||
for file in bxusb bxusb-gui prog-express; do |
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.
Could you move this one into src_prepare()
? All the preparations had better go there
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.
Thanks. I've pushed a new version and move that part into src_preare().
ca176dd
to
77fadcc
Compare
Pull Request assignment Areas affected: ebuilds, other files, profiles app-misc/prog-express: @gentoo/proxy-maint (new package) Bugs linked: 651396 In order to force reassignment and/or bug reference scan, please append |
891d21d
to
aefce93
Compare
ffd389f
to
e2cb835
Compare
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.36, Repoman-2.3.9 RepoMan-Options: --ignore-arches
Package-Manager: Portage-2.3.33, Repoman-2.3.9 Closes: gentoo#8204
Package-Manager: Portage-2.3.36, Repoman-2.3.9
EAPI 6 Package-Manager: Portage-2.3.36, Repoman-2.3.9
Uses deprecated EAPI 2. Package-Manager: Portage-2.3.36, Repoman-2.3.9
EAPI 6. Package-Manager: Portage-2.3.36, Repoman-2.3.9
Reported-by: rypervenche <contact@ryper.org> Closes: https://bugs.gentoo.org/602728 Package-Manager: Portage-2.3.36, Repoman-2.3.9
EAPI 6. Package-Manager: Portage-2.3.36, Repoman-2.3.9
Package-Manager: Portage-2.3.36, Repoman-2.3.9
Package-Manager: Portage-2.3.36, Repoman-2.3.9
Package-Manager: Portage-2.3.36, Repoman-2.3.9
Closes: https://bugs.gentoo.org/655472 Package-Manager: Portage-2.3.36, Repoman-2.3.9
Package-Manager: Portage-2.3.36, Repoman-2.3.9
Package-Manager: Portage-2.3.36, Repoman-2.3.9
Includes mozcoreconf-v6 and mozconfig-v6.60 eclasses Closes: http://bugs.gentoo.org/653678 Closes: http://bugs.gentoo.org/655396 Closes: http://bugs.gentoo.org/655022 Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
ESR 52.8.0 needs testing before stabilization Package-Manager: Portage-2.3.24, Repoman-2.3.6
Can be used as client for older bacula server installations. Bug: https://bugs.gentoo.org/651078 Package-Manager: Portage-2.3.36, Repoman-2.3.9
a40c533
to
6f48650
Compare
virtual/libusb:1 | ||
virtual/udev" | ||
|
||
RESTRICT="strip" |
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?
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.
License says: Restrictions: You are not allowed to modify Prog-Express itself or any parts of it nor apply any kind of reverse engineering to this software or any parts of it.
Shoudn't be strip
in that case be set?
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.
As with the other package, we generally don't assume stripping applies here. But it's your choice.
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.
Allright, dropped.
|
||
S="${WORKDIR}" | ||
|
||
DOCS=( "usr/share/doc/prog-express/changelog.gz" "usr/share/doc/prog-express/manuals" ) |
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.
Installing gzipped docs is not fine. Unpack it.
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.
Whoops, thanks.
# Fix lib path on amd64, since it's hardcoded to /usr/lib | ||
if use amd64; then | ||
for file in bxusb bxusb-gui prog-express; do | ||
sed -i -e "s/lib/$(get_libdir)/g" usr/bin/${file} || die |
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 aren't sed-ing binary files, are you? In any case, I don't see a reason to force lib64 here.
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.
No, this is not a binary, only a script. But I am removing it now, since I will only install to /usr/lib.
} | ||
|
||
src_install() { | ||
default |
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 does default
do here?
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.
Run einstalldocs?
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 it explicit.
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.
Removed by now. I looked again through it, einstalldocs shouldn't be needed at all in this case.
|
||
dosbin usr/sbin/bxfxload | ||
|
||
insinto /usr/$(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.
Any reason not to use usr/lib?
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.
No. I thought because of 17.1 profiles, I have to install in /usr/lib
and /usr/lib64
? Is this 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.
Both locations have their use cases.
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.
Ok, will use /usr/lib
.
Closes: https://bugs.gentoo.org/651396 Package-Manager: Portage-2.3.40, Repoman-2.3.9
licenses/prog-express
Outdated
|
||
Files: * | ||
Copyright: 2006-2015 Batronix Elektronik <support@batronix.com> | ||
License: Freeware |
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.
@gentoo/licenses, can we fit it with one of the existing licenses?
Pull request CI report Report generated at: 2018-06-20 09:19 UTC No issues found |
Depends: https://bugs.gentoo.org/651394
Closes: https://bugs.gentoo.org/651396