Hi, I am looking for a package review. * Description: Public Domain source code to the original DUNGEON game (Zork I). Released to the PD by Infocom. Includes source files, headers, and information. This version of Dungeon was modified from FORTRAN to C. The original was written in DEC FORTRAN, translated from MDL. It was then translated to f77 for UN*X systems, from which it was translated to C. The C translation was done with the help of f2c, the FORTRAN to C translator written by David Gay (AT&T Bell Labs), Stu Feldman (Bellcore), Mark Maimone (Carnegie-Mellon University), and Norm Schryer (AT&T Bell Labs). * Fedora Account System Username: jflory7 The package is also successfully built in COPR and Koji scratch builds. COPR URL: https://copr.fedorainfracloud.org/coprs/jflory7/zork/build/897220/ SPEC URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/zork.spec SRPM URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/zork-1.0.2-1.fc29.src.rpm
Fedora CFLAGS are not used during compilation, you can see it in the build.log: [...] cc -g -c -o ballop.o ballop.c cc -g -c -o actors.o actors.c cc -g -c -o demons.o demons.c cc -g -c -o clockr.o clockr.c cc -g -c -o dmain.o dmain.c cc -g -c dgame.c [...] Instead of setting environment variables before calling make, set them on make command line: %build %make_build \ CFLAGS="%{optflags}" \ DATADIR="%{_datadir}/%{name}" \ LDFLAGS="%{__global_ldflags}" %install %make_install \ BINDIR="%{buildroot}%{_bindir}" \ DATADIR="%{buildroot}%{_datadir}/%{name}/" \ LIBDIR="%{buildroot}%{_datadir}" \ MANDIR="%{buildroot}%{_mandir}" With that, your makefile patch can be shortened to: diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -42,7 +45,7 @@ TERMFLAG = # Uncomment the following line if you want to have access to the game # debugging tool. This is invoked by typing "gdt". It is not much # use except for debugging. -GDTFLAG = -DALLOW_GDT +# GDTFLAG = -DALLOW_GDT # Compilation flags CFLAGS = -g #-static @@ -69,7 +72,7 @@ dungeon: $(OBJS) dtextc.dat $(CC) $(CFLAGS) -o zork $(OBJS) $(LIBS) install: zork dtextc.dat - mkdir -p $(BINDIR) $(LIBDIR) $(MANDIR)/man6 + mkdir -p $(BINDIR) $(DATADIR) $(LIBDIR) $(MANDIR)/man6 cp zork $(BINDIR) cp dtextc.dat $(DATADIR) cp dungeon.6 $(MANDIR)/man6/ diff --git a/dinit.c b/dinit.c index d687cf4..cda5878 100644 --- a/dinit.c +++ b/dinit.c @@ -24,7 +24,7 @@ FILE *dbfile; #define TEXTFILE "lib:dtextc.dat" #else /* ! __AMOS__ */ #ifdef unix -#define TEXTFILE "/usr/games/lib/dunlib/dtextc.dat" +#define TEXTFILE "/usr/share/zork/dtextc.dat" #else /* ! unix */ I need a definition for TEXTFILE #endif /* ! unix */
Please include README.md as %doc, too.
Hi Dominik, thank you for reviewing. I pushed a new package release that addresses your feedback: COPR URL: https://copr.fedorainfracloud.org/coprs/jflory7/zork/build/898454/ SPEC URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/zork.spec SRPM URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/zork-1.0.2-2.fc29.src.rpm
Hi Dominik, I think this package is ready. Could you please give another look? Thank you.
1. The manpage is dungeon(6) while the binary is called "zork". This makes the manpage not discoverable easily. Please rename or at least add a link: %install ... echo ".so dungeon.6" > %{buildroot}%{_mandir}/man6/zork.6 %files ... %{_mandir}/man6/zork.6* 2. There are tons of warnings from gcc in the build log. Some of them look serious: https://copr-be.cloud.fedoraproject.org/results/jflory7/zork/fedora-rawhide-x86_64/00898454-zork/build.log.gz ... BUILDSTDERR: np.c:176:5: warning: array subscript -1 is outside array bounds of 'integer[40]' {aka 'int[40]'} [-Warray-bounds] BUILDSTDERR: 176 | --outbuf; BUILDSTDERR: | ^~~~~~~~ Please work with upstream to fix these. 3. The 'history' file could also be included as %doc. I missed that last time. Apart from that, it looks good packaging-wise and is APPROVED.
Ping?
SPEC URL: https://raw.githubusercontent.com/jwflory/rpmbuild/b82e9c1f8069c35f26827cd08b9e86cf4007d187/rpmbuild/SPECS/zork.spec SRPM URL: https://github.com/jwflory/rpmbuild/blob/b82e9c1f8069c35f26827cd08b9e86cf4007d187/rpmbuild/SRPMS/zork-1.0.2-3.fc31.src.rpm?raw=true So I finally got around to this. I was putting it off because I misread the gcc warnings as blocking the review. I opened the ticket, noticed the fedora-review flag was set, and felt quite silly. :-) Going to move on with packaging this. I pushed a new package version to address #1 and #3 of the above. I'll work on engaging with upstream to see if anything can be done about #2. Dropping the needsinfo and will request a dist-git repo.
Hi Dominik, could you please re-review or set the fedora-review flag once more? Today I learned there is a time quota on the fedora-review flag: $ fedpkg request-repo zork 1704522 Could not execute request_repo: The Bugzilla bug's review was approved over 60 days ago I'll get this packaged up once I can request the dist-git repo. Sorry to be a bother.
Approved again.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/zork
FEDORA-2020-fee0637f32 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-fee0637f32
FEDORA-EPEL-2020-7d28ec15d7 has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-7d28ec15d7
FEDORA-EPEL-2020-bd24cabef2 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-bd24cabef2
FEDORA-EPEL-2020-7d28ec15d7 has been pushed to the Fedora EPEL 8 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-7d28ec15d7 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2020-29d6c63c6c has been pushed to the Fedora EPEL 6 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-29d6c63c6c See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-fee0637f32 has been pushed to the Fedora 32 testing repository. In short time you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-fee0637f32 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-fee0637f32 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-32b0d57ff1 has been pushed to the Fedora 31 testing repository. In short time you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-32b0d57ff1 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-32b0d57ff1 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2020-bd24cabef2 has been pushed to the Fedora EPEL 7 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-bd24cabef2 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-2fbe3fb3b6 has been pushed to the Fedora 30 testing repository. In short time you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-2fbe3fb3b6 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-2fbe3fb3b6 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-fee0637f32 has been pushed to the Fedora 32 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2020-32b0d57ff1 has been pushed to the Fedora 31 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2020-2fbe3fb3b6 has been pushed to the Fedora 30 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2020-7d28ec15d7 has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2020-29d6c63c6c has been pushed to the Fedora EPEL 6 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2020-bd24cabef2 has been pushed to the Fedora EPEL 7 stable repository. If problem still persists, please make note of it in this bug report.