Bug 988997 - Review Request: bfgminer - A BitCoin miner
Review Request: bfgminer - A BitCoin miner
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-26 17:08 EDT by Paul Wouters
Modified: 2015-03-30 09:22 EDT (History)
14 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-12-19 10:04:08 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Paul Wouters 2013-07-26 17:08:38 EDT
Spec URL: ftp://ftp.nohats.ca/bfgminer/bfgminer.spec
SRPM URL: ftp://ftp.nohats.ca/bfgminer/bfgminer-3.1.3-1.fc20.src.rpm
Description: This is a multi-threaded multi-pool FPGA, GPU and CPU miner with ATI GPU monitoring, (over)clocking and fanspeed support for bitcoin and derivative coins.
Fedora Account System Username: pwouters
Comment 1 Peter Lemenkov 2013-07-27 04:01:41 EDT
Why blocking FE-LEGAL? I don't see any specific issues within the code - no problematic licensing, or Elliptic Curve Crypto usage. Actually bitcoin mining, contrary to bitcoin *usage*, doesn't involves any complex algorithms - it's just a sha256 calculation and comparison, so this shouldn't concern anyone.

Please elaborate your concerns before blindly trigger FE-LEGAL. Meanwhile I'm going to unblock it.
Comment 2 Christopher Meng 2013-07-27 23:48:49 EDT
Well, I'm not sure if thing related to bitcoin is approved now.
Comment 3 Peter Lemenkov 2013-07-28 01:03:18 EDT
(In reply to Christopher Meng from comment #2)
> Well, I'm not sure if thing related to bitcoin is approved now.

It was never disapproved. ECC on the other hand is still banned,
Comment 4 Paul Wouters 2013-08-05 06:51:08 EDT
I had not thought about the ECC bits. I will have to investigate if this application is using ECC or not. A quick grep does not show anything but we need to ensure there is no ECC in use here.
Comment 5 Scott Schmit 2013-10-15 06:32:42 EDT
Not saying "open the floodgates!" but ecc is no longer banned outright: bug 319901. CC'ing Tom.
Comment 6 Peter Lemenkov 2013-10-15 10:26:37 EDT
(In reply to Scott Schmit from comment #5)
> Not saying "open the floodgates!" but ecc is no longer banned outright: bug
> 319901. CC'ing Tom.

As I said already this application doesn't use ECC algorithms, and has nothing to do with OpenSSL. Bitcoin mining is different from actual bitcoin usage. Mining is just sha256 generation and comparison.

Just to stop further confusing I'm going to REVIEW this:

Koji scratchbuild for F-19 (just because building for F-19 is tenfold faster than for branches with ARM enabled)

* http://koji.fedoraproject.org/koji/taskinfo?taskID=6062348

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

- rpmlint is NOT silent

work ~/Desktop: rpmlint bfgminer-*
bfgminer.src: W: spelling-error %description -l en_US multi -> mulch, mufti
bfgminer.src: W: spelling-error %description -l en_US fanspeed -> fan speed, fan-speed, fans peed
bfgminer.src: W: spelling-error %description -l en_US bitcoin -> bit coin, bit-coin, bitchiness

^^^ False positives. Not a blocker.

bfgminer.src: W: no-version-in-last-changelog

^^^ Please fix that by adding date and version here before uploading to a git repository. Not a blocker.

bfgminer.src:50: W: macro-in-comment %dir
bfgminer.src:50: W: macro-in-comment %{_datadir}
bfgminer.src:56: W: macro-in-comment %{_mandir}
bfgminer.src:56: W: macro-in-comment %{name}

^^^ Either remove commented lines or escape them with additional percent sign. Not a blocker.

bfgminer.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
bfgminer.x86_64: W: spelling-error %description -l en_US fanspeed -> fan speed, fan-speed, fans peed
bfgminer.x86_64: W: spelling-error %description -l en_US bitcoin -> bit coin, bit-coin, bitchiness

^^^ Likewise, false positives. Not a blocker.

bfgminer.x86_64: W: no-version-in-last-changelog


^^^ Likewise, fix that by adding date and version here before uploading to a git repository. Not a blocker.

bfgminer.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/bfgminer-3.1.3/api-example.o

^^^ Please remove this. That's a blocker.

bfgminer.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/bfgminer ['$ORIGIN/libblkmaker/.libs']

^^^ rpath issue. Please fix it. That's a blocker.

bfgminer.x86_64: E: library-without-ldconfig-postin /usr/lib64/libblkmaker_jansson-0.1.so.0.4.0
bfgminer.x86_64: E: library-without-ldconfig-postun /usr/lib64/libblkmaker_jansson-0.1.so.0.4.0
bfgminer.x86_64: E: library-without-ldconfig-postin /usr/lib64/libblkmaker-0.1.so.0.4.0
bfgminer.x86_64: E: library-without-ldconfig-postun /usr/lib64/libblkmaker-0.1.so.0.4.0

^^^ Please, call ldconfig where necessary. That's a blocker.

bfgminer.x86_64: W: no-manual-page-for-binary bitforce-firmware-flash
bfgminer.x86_64: W: no-manual-page-for-binary bfgminer
bfgminer.x86_64: W: no-manual-page-for-binary bfgminer-rpc

^^^ Sad truth. These binaries just doesn't have a corresponding man-page. Not a blocker.

bfgminer-debuginfo.x86_64: W: no-version-in-last-changelog
bfgminer-devel.x86_64: W: no-version-in-last-changelog

^^^ Fix that by adding date and version here before uploading to a git repository. Not a blocker.

bfgminer-devel.x86_64: W: no-documentation

^^^ That was intended - this sub-package doesn't have any docs.

4 packages and 0 specfiles checked; 6 errors, 18 warnings.
work ~/Desktop:

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

- The package DOESN'T fully conform to the Packaging Guidelines. See my notes above.

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv3 exactly).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.

- The package stores shared library files in some of the dynamic linker's default paths, so it MUST call ldconfig in %post and %postun. See my notes regarding ldconfig above.

+/- The package does NOT bundle copies of system libraries. Well, to be honest it does contains some sources from the libraries unknown to me within the ADL, CL, lib directories which *should* be unbundled but I'm not insisting on doing it right now. However that would be great to re-use system-wide opencl-headers package instead of shipped CL.

0 The package is not designed to be relocatable.

- The package MUST own all directories that it creates. Hewever I see that /usr/share/bfgminer directory left unowned. Please add it back (it seems that you accidentally commented it out) maybe with %dir mark to prevent files listing twice.

+ The package does not list a file more than once in the spec file's %files listings. See above note regarding directories.
+ Permissions on files are set properly.
+ The package consistently uses macros.
+ The package contains code, or permissible content. Note it contains firmware blobs in the bitstreams/ directory.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files are stored in a -devel package.
0 No static libraries.
+ The pkgconfig(.pc) files are stored in a -devel package and necessary runtime requirement picked up automatically.
+ The library file(s) that end in .so (without suffix) is(are) stored in a -devel package.

- The -devel package MUST require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release}

+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ All filenames in rpm packages are valid UTF-8.


OK, almost there. Please fix/comment issues mentioned above and I'll continue.
Comment 7 Warren Togami 2013-10-15 20:29:43 EDT
Please verify the license of the ADL files.  Are they really open source?
Comment 8 Peter Lemenkov 2013-10-16 01:26:35 EDT
(In reply to Warren Togami from comment #7)
> Please verify the license of the ADL files.  Are they really open source?

Yes, they are. They made by the author of bfgminer, and licensed under MIT.
Comment 9 luke-jr+redhatbugs 2013-10-21 16:43:18 EDT
ADL/ is, as already mentioned, custom ADL-compatible headers written by myself and provided under a MIT license.

CL/ is the official OpenCL 1.0 headers. Newer headers have created compatibility issues on some platforms. If you want to use system headers, I suggest having the build rm the directory and replace it with a symlink.

lib/ is gnulib snippets, which are unfortunately not designed such that they can be shared. I'd be glad to make this a proper dependency if you can get gnulib to make that possible.

libblkmaker/ can be de-bundled to another package, if the maintainers want. configure accepts --with-system-libblkmaker for such a case.

bfgminer-devel is I presume only libblkmaker headers right now. BFGMiner itself does not have headers or other devel files, but does have a HACKING documentation file for driver development.
Comment 10 Christopher Meng 2013-10-21 18:48:09 EDT
gnulib can be bundled,  see:

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
Comment 11 Paul Wouters 2013-10-21 20:50:19 EDT
Spec URL: ftp://ftp.nohats.ca/bfgminer/bfgminer.spec
SRPM URL: ftp://ftp.nohats.ca/bfgminer/bfgminer-3.1.3-2.fc19.src.rpm

Changelog:

* Mon Oct 21 2013 Paul Wouters <pwouters@redhat.com> - 3.1.3-2
- Fixed version number in changelog
- Removed accidental install of api-example.o
- Run ldconfig in post and postun
- Make devel depend on arch


This fixes all issues but the rpath issue with libblkmaker. I'm planning to submit a package review for that package separately. That might take a few days, while I figure out what to do with https://gitorious.org/bitcoin/libblkmaker not having releases, only tags.

So until that package is reviewed and accepted, I'll wait with bfgminer.

Thanks everyone
Comment 12 Peter Lemenkov 2013-10-22 00:18:07 EDT
Lu(In reply to luke-jr+redhatbugs from comment #9)
> ADL/ is, as already mentioned, custom ADL-compatible headers written by
> myself and provided under a MIT license.
> 
> CL/ is the official OpenCL 1.0 headers. Newer headers have created
> compatibility issues on some platforms. If you want to use system headers, I
> suggest having the build rm the directory and replace it with a symlink.
> 
> lib/ is gnulib snippets, which are unfortunately not designed such that they
> can be shared. I'd be glad to make this a proper dependency if you can get
> gnulib to make that possible.
> 
> libblkmaker/ can be de-bundled to another package, if the maintainers want.
> configure accepts --with-system-libblkmaker for such a case.
> 
> bfgminer-devel is I presume only libblkmaker headers right now. BFGMiner
> itself does not have headers or other devel files, but does have a HACKING
> documentation file for driver development.

Thanks for the explanation, Luke! As Christopher mentioned above, gnulib is not an issue at all. As for libblkmaker - I don't see unbundling it as a requirement since it's not available in Fedora as a standalone package right now. Perhaps with time when someone packages it we'll reconsider libblkmaker situation.

OK, Paul, I don't see any other issues. I'd like to remind you to add "Provides: bundled(gnulib)" before uploading to git, as documented here:

* https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Packages_granted_exceptions

As I said I don't see any other issues. so this package is




APPROVED.
Comment 13 Christopher Meng 2013-10-22 00:49:23 EDT
NOTE:

ADD "Provides:       bundled(gnulib)" in your spec.

Ref: bug #821770
Comment 14 luke-jr+redhatbugs 2013-10-22 01:22:04 EDT
(In reply to Paul Wouters from comment #11)
> https://gitorious.org/bitcoin/libblkmaker not having releases, only tags.

Tags are releases.

But this may be irrelevant considering Peter's comment that it does not need to be split.
Comment 15 Paul Wouters 2013-10-22 19:25:13 EDT
I removed the rpath, although unfortunately I had to resort to the chrpath method as removng in from Makefile.in weirdly changes the .la files to be .lai and fail and I couldn't figure out why.

Spec URL: ftp://ftp.nohats.ca/bfgminer/bfgminer.spec
SRPM URL: ftp://ftp.nohats.ca/bfgminer/bfgminer-3.1.3-3.fc19.src.rpm
Comment 16 Paul Wouters 2013-10-22 19:27:04 EDT
New Package SCM Request
=======================
Package Name: bfgminer
Short Description: A BitCoin miner
Owners: pwouters
Branches: f19 f20 el6
InitialCC:
Comment 17 luke-jr+redhatbugs 2013-10-22 19:36:35 EDT
Why 3.1.3? That's relatively ancient, has known bugs, and unsupported..
Comment 18 Paul Wouters 2013-10-22 22:12:28 EDT
oops. I guess we all missed upstream had released new versions :)

I've packaged it up.


Spec URL: ftp://ftp.nohats.ca/bfgminer/bfgminer.spec
SRPM URL: ftp://ftp.nohats.ca/bfgminer/bfgminer-3.3.0-1.fc19.src.rpm


There are two patches now. One for rpath and one for the chdir+setgroups calls. 

Luke: you can find the two patches here:

ftp://ftp.nohats.ca/bfgminer/bfgminer-3.3.0-configure.patch
ftp://ftp.nohats.ca/bfgminer/bfgminer-3.3.0-privs.patch

But I also notice the bitstreams firmware blobs are no longer shipped. While I see the source for them, I'm unfamiliar with how to compile those. I guess I can package it in this package as blobs, like it was. Or make a separate package and make this package depend on it.
Comment 19 luke-jr+redhatbugs 2013-10-22 22:17:14 EDT
The firmware blobs are only needed for specific devices; I doubt most users would want them to be installed by default/requirement. README.FPGA should have links to the firmware authors' websites with the binaries.
Comment 20 luke-jr+redhatbugs 2013-10-22 22:17:49 EDT
Regarding the first patch, it breaks running from the build directory...
Comment 21 luke-jr+redhatbugs 2013-10-22 22:47:57 EDT
Regarding the second, setgroups is not a POSIX standard function, so will need configure checks at least. It also seems less preferable than initgroups. Both of these require _BSD_SOURCE and/or -lbsd-compat on some platforms.
Comment 22 Gwyn Ciesla 2013-10-23 08:40:07 EDT
Git done (by process-git-requests).
Comment 23 Peter Lemenkov 2013-11-03 06:34:30 EST
Ping, Paul! How're things going?
Comment 24 zombie214 2013-12-18 04:06:01 EST
Paul Wouters, 

Using our package on F19/20 I see the following error in the build log, stemming from line 81 in file 'missing'

/builddir/build/BUILD/bfgminer-3.3.0/missing aclocal-1.13 -I m4
/builddir/build/BUILD/bfgminer-3.3.0/missing: line 81: aclocal-1.13: command not found
WARNING: 'aclocal-1.13' is missing on your system.

For some strange reason (path during build maybe?) aclocal-1.13 is not found, though I have confirmed that aclocal-1.13 is indeed on my system (F20).

Also, if you google for 

"aclocal-1.13 line 81 missing" 

you will dozens of builds that fail right at this point - missing seems to be flawed.
Comment 25 zombie214 2013-12-18 21:39:46 EST
The aclocal-1.13 not found (see comment 24) have been fixed by Rex Dieters http://pkgs.fedoraproject.org/cgit/bfgminer.git/commit/?id=43b828eb84f6e94e1ee2022475480c2cb96b356f
Comment 26 luke-jr+redhatbugs 2013-12-19 10:35:35 EST
Is there any way to get this updated to a supported branch? 3.2.x and 3.5.x are currently receiving backports, but 3.3.0 is very dead by now.
Comment 27 Paul Wouters 2015-03-30 09:22:01 EDT
I have orphaned this package - someone else is free to pick it up again.

Note You need to log in before you can comment on or make changes to this bug.