[Bug 1325378] Review Request: spasm-ng - A z80 assembler with extra features for TI calculators

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1325378



--- Comment #8 from gil cattaneo <puntogil@xxxxxxxxx> ---
Issues:


[reply] [−] Description Ben Rosser 2016-04-08 11:40:44 EDT
Spec URL: https://tc01.fedorapeople.org/spasm/spasm-ng.spec
SRPM URL:
https://tc01.fedorapeople.org/spasm/spasm-ng-0.5-0.2.beta.2.fc23.src.rpm

Description:
SPASM-ng is a z80 assembler with extra features to support development
for TI calculators. SPASM-ng can assemble and create assembly programs
and flash applications in formats that can be shipped directly to
TI-z80 (TI-83+, TI-83+SE, TI-84+, TI-83+SE, TI-84+CSE, TI-84+CE)
calculators.

SPASM-ng was originally from the SPASM project, and was forked to fix
a few bugs. It was originally written by Spencer Putt and Don Straney,
with additional development by Chris Shappell and James Montelongo.

This release incorporates eZ80 support in preparation for the launch
of the TI-84+CE. It also greatly increases the limit on the number
of labels that can be defined.

Fedora Account System Username: tc01

Some notes:

* spasm-ng is available in this COPR
(https://copr.fedorainfracloud.org/coprs/tc01/spasm-ng/) if you wish to test
it.

* spasm-ng is distributed upstream alongside some z80 assembly include files,
for linking against the TI-83+. Where should these be distributed? I've put
them into /usr/include/spasm-ng, which feels like... not quite the right place,
but I'm not sure if .../share/ makes any more sense.

* One of these files
(https://github.com/alberthdev/spasm-ng/blob/master/inc/ti83plus.inc) was
originally written by TI, and I'm not certain we (Fedora) can legally
redistribute it. If necessary, it can be removed from the package. On the other
hand, the included license text seems only to establish the file is under no
warranty and that its copyright notice be left intact, and the calculator
hacking community has redistributed this file for years. (Though that obviously
doesn't mean Fedora can ship it).
[reply] [−] Comment 1 Ben Rosser 2016-07-19 19:53:22 EDT
Having thought about the include file situation further... I believe they
probably should be removed from the package:

Their authorship and copyright is difficult to assert (most of the community
written files are not licensed, and ti83plus.inc was originally written by TI
and then heavily modified), most authors of TI calculator assembly projects
bundle them with their code anyway, and spasm does not expect include files to
be available system-wide to begin with. The AUR package for Arch makes a
similar decision and does not include them.

I have also realized the package was not being built with system-wide ldflags,
so I fixed that and reuploaded the package.

Spec URL: https://tc01.fedorapeople.org/spasm/spasm-ng.spec
SRPM URL:
https://tc01.fedorapeople.org/spasm/spasm-ng-0.5-0.3.beta.2.fc23.src.rpm

I believe it is now ready for review.
[reply] [−] Comment 2 gil cattaneo 2016-08-27 04:47:27 EDT
can you take this https://bugzilla.redhat.com/show_bug.cgi?id=1366839 for me?
Assignee: nobody@xxxxxxxxxxxxxxxxx → puntogil@xxxxxxxxx
Flags: fedora-review?
[reply] [−] Comment 3 gil cattaneo 2016-08-27 04:50:15 EDT
Please, remove rm -rf %{buildroot} in %install section
[reply] [−] Comment 4 gil cattaneo 2016-08-27 04:56:49 EDT
I get Error 404 downloading
https://tc01.fedorapeople.org/spasm/spasm-ng-0.5-0.3.beta.2.fc23.src.rpm
please, fix also the problem reported above
[reply] [−] Comment 5 Ben Rosser 2016-08-27 12:47:12 EDT
Oh, whoops. The correct SRPM is fc24, not fc23:

Spec URL: https://tc01.fedorapeople.org/spasm/spasm-ng.spec
SRPM URL:
https://tc01.fedorapeople.org/spasm/spasm-ng-0.5-0.3.beta.2.fc24.src.rpm

I will fix the rm -rf %{buildroot} as well.
[reply] [−] Comment 6 gil cattaneo 2016-08-27 12:58:38 EDT
(In reply to Ben Rosser from comment #5)

> I will fix the rm -rf %{buildroot} as well.

It is still there ...
[reply] [−] Comment 7 gil cattaneo 2016-08-27 13:22:02 EDT
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[!]: Package contains no static executables.

 ./lib/x64/mpir.lib
 ./lib/x64/mpir.pdb
 ./lib/mpir.lib
 ./lib/mpir.pdb
 Please, remove

[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (3 clause)", "LGPL (v2.1 or later) (with incorrect FSF
     address)", "GPL (v2 or later)", "Unknown or generated". 53 files have
     unknown license. Detailed output of licensecheck in /home/gil/1325378
     -spasm-ng/licensecheck.txt

 ./gmp.h LGPL (v2.1 or later) (with incorrect FSF address)
 ./modp_ascii.cpp BSD ./modp_ascii.h (3 clause)
 The remain source code is without license headers.
 Please, report the problem to upstream and ask to add the license header where
are missing

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#License_Clarification

[!]: Package contains no bundled libraries without FPC exception.

 Please, add:
 GNU MP Library ./gmp.h
 Provides:   bundled(gmp) = [unknown version]
 http://code.google.com/p/stringencoders/ ./modp_ascii.cpp BSD ./modp_ascii.h
 Provides:   bundled(stringencoders) = [unknown version]
 i dont known the version this libraries. Please replace "[unknown version]"
with an appropriate value

[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]