Bug 1535549 - Review Request: mupen64plus - Nintendo 64 Emulator
Summary: Review Request: mupen64plus - Nintendo 64 Emulator
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-01-17 15:35 UTC by Wade Berrier
Modified: 2020-06-11 22:56 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-06-11 18:58:46 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Wade Berrier 2018-01-17 15:35:15 UTC
Spec URL: http://berrier.org/tmp/mupen64plus.spec
SRPM URL: http://berrier.org/tmp/mupen64plus-2.5-2.fc27.src.rpm
Description: Nintendo 64 Emulator
Fedora Account System Username: wberrier

Comment 1 Robert-André Mauchin 🐧 2018-01-17 17:21:42 UTC
 - Not needed in Fedora:

Group:

BuildRoot:

%defattr(***, root, root) in %files


 - .desktop files must be validated in %install or %check. See https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

desktop-file-validate %{buildroot}/%{_datadir}/applications/mupen64plus.desktop

 - Since you are installing icons in hicolor, you should Requires:       hicolor-icon-theme

 - In the -devel subpackage, you should provide an unversioned shared library file linking to libmupen64plus.so.2.0.0. Just create the symlink in %install

ln -sf %{_libdir}/libmupen64plus.so.2.0.0 %{buildroot}%{_libdir}/libmupen64plus.so

   And include it in -devel %files:

%files devel
%{_includedir}/mupen64plus/
%{_libdir}/libmupen64plus.so

 - There's various LICENSES files included in the source, add them all to %license. In order not to overwrite the same file, you need to rename them first.

   In %prep:

cp -a source/mupen64plus-rsp-hle/LICENSES LICENSE-rsp-hle
cp -a source/mupen64plus-rom/mupen64plus/assets/LICENSES LICENSE-assets
cp -a source/mupen64plus-rom/LICENSES LICENSE-rom
cp -a source/mupen64plus-input-sdl/LICENSES LICENSE-input-sdl
cp -a source/mupen64plus-video-glide64mk2/LICENSES LICENSE-video-glide64mk2
cp -a source/mupen64plus-video-rice/LICENSES LICENSE-video-rice
cp -a source/mupen64plus-ui-console/LICENSES LICENSE-ui-console
cp -a source/mupen64plus-core/LICENSES LICENSE-core
cp -a source/mupen64plus-audio-sdl/LICENSES LICENSE-audio-sdl

  In %files:

%files
%license LICENSE-rsp-hle LICENSE-assets LICENSE-rom LICENSE-input-sdl LICENSE-video-glide64mk2 LICENSE-video-rice LICENSE-core LICENSE-audio-sdl

 - The assets in source/mupen64plus-rom/mupen64plus/assets/ are actually Creative Common Attribution-ShareAlike 3.0, so you should add that license to the License: field:

License:	GPLv2+ and CC-BY-SA

 - Since you're including a shared library, you must run ldconfig in %post and %postun:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

   See https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries

 - You should own this directory: /usr/lib64/mupen64plus :

%dir %{_libdir}/%{name}

 - In the %changelog, put a space before the version info, otherwise it's not recognized correctly:

* Thu Jan 11 2018 Wade Berrier <wberrier> - 2.5-2

 - Use pkgconfig for your devel deps when you can:

BuildRequires:	pkgconfig(SDL_ttf)
BuildRequires:	pkgconfig(lirc)
BuildRequires:	desktop-file-utils
BuildRequires:	pkgconfig(glu)
BuildRequires:	pkgconfig(samplerate)
BuildRequires:	pkgconfig(libpng)
BuildRequires:	pkgconfig(sdl)
BuildRequires:	pkgconfig(freetype2)
BuildRequires:	boost-devel
BuildRequires:	gzip
BuildRequires:	pkgconfig(glew)
BuildRequires:	binutils



Package Review
==============

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


Issues:
=======
- ldconfig called in %post and %postun if required.
  Note: /sbin/ldconfig not called in mupen64plus
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries
- Package installs a %{name}.desktop using desktop-file-install or desktop-
  file-validate if there is such a file.


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[!]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL", "GPL (v2 or later)", "Unknown or generated", "MIT/X11
     (BSD like)", "zlib/libpng", "BSD (2 clause)", "GPL (v2 or later) (with
     incorrect FSF address)", "GPL (with incorrect FSF address)", "LGPL
     (v2.1 or later)", "BSD (3 clause) GPL (v2 or later)", "GPL (v2)". 119
     files have unknown license. Detailed output of licensecheck in
     /home/bob/packaging/review/mupen64plus/review-
     mupen64plus/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/lib64/mupen64plus
[!]: Package must own all directories that it creates.
     Note: Directories without known owners:
     /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/48x48/apps,
     /usr/lib64/mupen64plus, /usr/share/icons/hicolor/scalable/apps,
     /usr/share/icons/hicolor, /usr/share/icons/hicolor/scalable
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[!]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[!]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Buildroot: present but not needed
[-]: Avoid bundling fonts in non-fonts packages.
     Note: Package contains font files
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[!]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define debug_package %{nil}
[x]: Reviewer should test that the package builds in mock.
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 2007040 bytes in /usr/share
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: mupen64plus-2.5-2.fc28.x86_64.rpm
          mupen64plus-devel-2.5-2.fc28.x86_64.rpm
          mupen64plus-2.5-2.fc28.src.rpm
mupen64plus.x86_64: E: explicit-lib-dependency libpng
mupen64plus.x86_64: E: explicit-lib-dependency libsamplerate
mupen64plus.x86_64: W: incoherent-version-in-changelog -2.5-2 ['2.5-2.fc28', '2.5-2']
mupen64plus.x86_64: E: library-without-ldconfig-postin /usr/lib64/libmupen64plus.so.2.0.0
mupen64plus.x86_64: E: library-without-ldconfig-postun /usr/lib64/libmupen64plus.so.2.0.0
mupen64plus.x86_64: E: script-without-shebang /usr/share/icons/hicolor/scalable/apps/mupen64plus.svg
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-assets
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-audio-sdl
mupen64plus.x86_64: E: incorrect-fsf-address /usr/share/licenses/mupen64plus/LICENSE-audio-sdl
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-core
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-input-sdl
mupen64plus.x86_64: E: incorrect-fsf-address /usr/share/licenses/mupen64plus/LICENSE-input-sdl
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-rom
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-rsp-hle
mupen64plus.x86_64: E: incorrect-fsf-address /usr/share/licenses/mupen64plus/LICENSE-rsp-hle
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-video-glide64mk2
mupen64plus.x86_64: E: incorrect-fsf-address /usr/share/licenses/mupen64plus/LICENSE-video-glide64mk2
mupen64plus.x86_64: E: script-without-shebang /usr/share/licenses/mupen64plus/LICENSE-video-rice
mupen64plus.x86_64: E: incorrect-fsf-address /usr/share/licenses/mupen64plus/LICENSE-video-rice
mupen64plus.x86_64: W: spurious-executable-perm /usr/share/man/man6/mupen64plus.6.gz
mupen64plus.x86_64: E: script-without-shebang /usr/share/mupen64plus/Glide64mk2.ini
mupen64plus.x86_64: E: script-without-shebang /usr/share/mupen64plus/InputAutoCfg.ini
mupen64plus.x86_64: E: script-without-shebang /usr/share/mupen64plus/RiceVideoLinux.ini
mupen64plus.x86_64: E: script-without-shebang /usr/share/mupen64plus/mupen64plus.ini
mupen64plus.x86_64: E: script-without-shebang /usr/share/mupen64plus/mupencheat.txt
mupen64plus-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 23 errors, 3 warnings.

Comment 2 Robert-André Mauchin 🐧 2018-01-17 17:28:17 UTC
I don't see you in the packager group, you probably need to find a sponsor. See https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group for more info. Introduce yourself on the fedora-devel mailing list is also a good idea. You should mention that you are already a packager for RPMFusion or ask your sponsor there to also sponsor you for Fedora.

Comment 3 Antonio T. (sagitter) 2018-01-18 18:40:04 UTC
@Robert-André

Please, let me care this package; i have followed it from rpmfusion.

@Wade

Fix all issues pointed by Robert-André, then we can go on with the review.

Comment 4 Robert-André Mauchin 🐧 2018-01-18 22:02:36 UTC
As you wish Antonio.

Comment 5 Wade Berrier 2018-01-20 20:50:32 UTC
Robert-André, thanks for the comprehensive review.

I believe I've made all of the requested changes, and have new updates here:

Spec URL: http://berrier.org/tmp/mupen64plus.spec
SRPM URL: http://berrier.org/tmp/mupen64plus-2.5-3.fc27.src.rpm

I also did some additional cleanups with BuildRequires and Requires, and did a test mock build.

Question: I changed "%define debug_package" to use "global", is that correct?  What do I need to do about a debuginfo package?

NOTE: rpmlint is warning about libsamplerate and libpng, but "rpm -qp --requires" doesn't list those dependencies, where as ldd /usr/lib64/mupen64plus/* does show those dependencies.  So, I left those Requires in.  Not sure what to do about that.

Comment 6 Robert-André Mauchin 🐧 2018-01-20 21:39:33 UTC
You shouldn't patch the FSF address in the LICENSES files. It is allowed when it's just in the headers of a source file, but not for license files. See: https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address

Comment 7 Robert-André Mauchin 🐧 2018-01-20 23:25:52 UTC
>Question: I changed "%define debug_package" to use "global", is that correct?

   Yes

>What do I need to do about a debuginfo package?

   Indeed, the best would be to generate the debug infos but whatever I try, it still fails.

   Anyhow you still should enable Fedora's default flags for the build:

%build
export CFLAGS="%{optflags}"
export CXXFLAGS="%{optflags}"
export LDFLAGS="%{?__global_ldflags}"
sh m64p_build.sh LIRC=1

  - Another issue is that the build script install the libraries without the executable bits. I thought it was what's causing the debug info not to be generated (binaries must be executable for find-debuginfo to work) but that's still not it. Still set the executable bits for those:

%install

# NOTE: set LDCONFIG to true so it's not run during this script
./m64p_install.sh DESTDIR=%{buildroot} PREFIX=%{_prefix} MANDIR=%{_mandir} LIBDIR=%{_libdir} LDCONFIG='true'
find %{buildroot}%{_libdir} -type f -name "*.so*" -exec chmod 0755 "{}" \;


 - You don't need the following Requires:

Requires:	SDL_ttf
Requires:	mesa-libGLU
Requires:	libsamplerate
Requires:	libpng
Requires:	freetype
Requires:	boost

   They are correctly detected automatically, see:

$ rpm -q --requires -p mupen64plus-2.5-3.fc28.x86_64.rpm   | sort -f | uniq -c
      2 /sbin/ldconfig
      1 hicolor-icon-theme
      1 libboost_filesystem.so.1.64.0()(64bit)
      1 libboost_system.so.1.64.0()(64bit)
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.11)(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libc.so.6(GLIBC_2.7)(64bit)
      1 libdl.so.2()(64bit)
      1 libdl.so.2(GLIBC_2.2.5)(64bit)
      1 libfreetype.so.6()(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libgcc_s.so.1(GCC_3.3.1)(64bit)
      1 libGL.so.1()(64bit)
      1 libGLU.so.1()(64bit)
      1 liblirc_client.so.0()(64bit)
      1 libm.so.6()(64bit)
      1 libm.so.6(GLIBC_2.15)(64bit)
      1 libm.so.6(GLIBC_2.2.5)(64bit)
      1 libpng16.so.16()(64bit)
      1 libpng16.so.16(PNG16_0)(64bit)
      1 libpthread.so.0()(64bit)
      1 libsamplerate.so.0()(64bit)
      1 libsamplerate.so.0(libsamplerate.so.0.0)(64bit)
      1 libsamplerate.so.0(libsamplerate.so.0.1)(64bit)
      1 libSDL2-2.0.so.0()(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(CXXABI_1.3.8)(64bit)
      1 libstdc++.so.6(CXXABI_1.3.9)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.15)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
      1 libz.so.1()(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)

Comment 8 Robert-André Mauchin 🐧 2018-01-21 00:10:12 UTC
   Ah I got it! I noticed there is a debug option in the Makefile:

Debugging Options:"
	@echo "    PROFILE=1     == build gprof instrumentation into binaries for profiling"
	@echo "    DEBUG=1       == add debugging symbols to binaries"


   Of course I tried to add it to the build line, with no success. It's not needed there anyway since it's already taken care of by the default Fedora %optflags.

   However at install time, this debug option enables or disables a "-s" flag for install. And what does "install -s" do? It strips the binaries, thus preventing find-debuginfo to find the debugging symbols.

   TL;DR: add DEBUG=1 to the install line:

# NOTE: set LDCONFIG to true so it's not run during this script
./m64p_install.sh DESTDIR=%{buildroot} PREFIX=%{_prefix} MANDIR=%{_mandir} LIBDIR=%{_libdir} DEBUG=1 LDCONFIG='true


   And voilà the debug packages are built.

Comment 9 Antonio T. (sagitter) 2018-01-21 10:21:47 UTC
I'm leaving this package to Robert-André, who cannot do without comment this ticket.

Comment 10 Wade Berrier 2018-01-28 04:11:31 UTC
Ok, I've made the additional changes and posted them here:

http://berrier.org/tmp/mupen64plus.spec
http://berrier.org/tmp/mupen64plus-2.5-4.fc27.src.rpm

(In reply to Robert-André Mauchin from comment #6)
> You shouldn't patch the FSF address in the LICENSES files. It is allowed
> when it's just in the headers of a source file, but not for license files.
> See:
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address

So, what to do about this?  In the meantime I've removed the patch.

Comment 11 Robert-André Mauchin 🐧 2018-01-28 14:56:43 UTC
> So, what to do about this? 

Just notify upstream about it.


Package is approved.

Comment 12 Wade Berrier 2018-02-03 16:29:32 UTC
Upstream notified about the incorrect addresses in the license files:

https://github.com/mupen64plus/mupen64plus-core/issues/527

Git repo requests:

f27:

https://pagure.io/releng/fedora-scm-requests/issue/4359

f26:

https://pagure.io/releng/fedora-scm-requests/issue/4360

I've been following the instructions at https://fedoraproject.org/wiki/Package_Review_Process

Anything else other than what's in there that I need to do?

Comment 13 Wade Berrier 2018-02-03 20:21:12 UTC
I guess I need a sponsor?

My pagure.io requests were closed.

Comment 14 Robert-André Mauchin 🐧 2019-03-18 22:33:02 UTC
(In reply to Wade Berrier from comment #13)
> I guess I need a sponsor?
> 
> My pagure.io requests were closed.

Yes you need a sponsor: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Comment 15 Geoffrey Marr 2019-07-22 17:15:40 UTC
Wade, were you ever sponsored into the packagers group?

Comment 16 Wade Berrier 2019-11-23 15:56:00 UTC
No, I don't think I was.

Comment 17 Robert-André Mauchin 🐧 2019-12-03 22:10:18 UTC
(In reply to Wade Berrier from comment #16)
> No, I don't think I was.

Sent you a mail regarding that.

Comment 18 Wade Berrier 2019-12-28 17:56:11 UTC
(In reply to Robert-André Mauchin from comment #17)
> (In reply to Wade Berrier from comment #16)
> > No, I don't think I was.
> 
> Sent you a mail regarding that.

I responded with an email with the sponsorship mock packaging review.

Comment 19 Robert-André Mauchin 🐧 2020-01-06 16:02:28 UTC
Sponsored.

Comment 20 Robert-André Mauchin 🐧 2020-01-06 16:02:59 UTC
Refreshed flag.

Comment 21 Wade Berrier 2020-04-12 17:26:07 UTC
What's the next step to get this accepted?

Comment 22 David Auer 2020-04-12 19:59:32 UTC
The package is accepted and you are sponsored, so the next step would be to request a git repository and branches. We actually have two wiki pages describing the process, I'm not sure which one is better so I'll link both:

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
https://fedoraproject.org/wiki/Package_Review_Process#Contributor

The version 2.5 from 2015 is still the latest release by upstream so nothing to be updated here. (Side note: They do have a pre-release for 2.6 called v2.5.9 which is now over a year old. To me, they seem to be releasing quite rarely considering the commit activity of the repository, wonder why that is.)

Comment 23 David Auer 2020-04-12 20:02:53 UTC
Dang, I tried to reset the review flag but I can't, now you have to wait Robert-André fixes that. Sorry but I think it was outdated anyway.

Comment 24 Wade Berrier 2020-04-13 00:45:48 UTC
Sounds good.  I ran the `request-repo` command but it responded with:

Could not execute request_repo: The Bugzilla bug is not approved yet

I guess that's what you mean about the review flag.

Comment 25 Robert-André Mauchin 🐧 2020-04-13 18:07:36 UTC
Refreshed flag

Comment 26 Gwyn Ciesla 2020-04-13 18:30:40 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/mupen64plus

Comment 27 David Auer 2020-05-23 21:27:13 UTC
Hey, what is the status here? I can see the repository is set up but hasn't any successful builds yet. I am interested in using and maybe helping / co-maintaining this package since I like to play some old games every now and then.

I tried a build [1] but it failed with a linking error [2]. Haven't put that into Google yet - have you already seen or fixed that by any chance, Wade?


>    LD  libmupen64plus.so.2.0.0
>/usr/bin/ld: _obj/main/savestates.o (symbol from plugin): in function `savestates_select_slot':
>(.text+0x0): multiple definition of `work'; _obj/api/frontend.o (symbol from plugin):(.text+0x0): first defined here
>/usr/bin/ld: _obj/main/workqueue.o (symbol from plugin): in function `workqueue_init':
>(.text+0x0): multiple definition of `work'; _obj/api/frontend.o (symbol from plugin):(.text+0x0): first defined here
>collect2: error: ld returned 1 exit status


Should we have this discussion here or should I open a new issue since the package review is technically done and this is rather a build issue?

On another topic I hope that upstream can be persuaded to make a release every now an then, they are discussing/planning 2.6+ since 2017 [3]. As far as I know we can not just go ahead and package their git master in Fedora so that would restrict us to a pretty old code base.

1: https://koji.fedoraproject.org/koji/taskinfo?taskID=44875948
2: https://kojipkgs.fedoraproject.org//work/tasks/6033/44876033/build.log (from the x86_64 build)
3: https://github.com/mupen64plus/mupen64plus-core/issues/353

Comment 28 Wade Berrier 2020-05-23 23:21:57 UTC
I had this building on f31, but I think what's going on is that gcc is stricter about duplicate symbols in f32.

It's on my list of things to do, but I haven't gotten to it yet.

I'd be happy to have you co-maintain this (although I'm not sure how to set that up).

Comment 29 David Auer 2020-05-23 23:58:59 UTC
We can instruct ld to allow multiple definitions, then the build works, but should we?

Builds, not tested: 
https://src.fedoraproject.org/fork/dreua/rpms/mupen64plus/c/046ba6166ed6e25d084043e12b157e9f13350c72?branch=fix-ld-muldefs

Maybe this fix from gentoo is better:
https://bugs.gentoo.org/708054
https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=447ad3ceef7905cc45267886632da13f6a2bc8ed

Btw, maybe we can save us some time by looking what patches other distributions use since the release is quite old.

It's getting late here and I can't test the build right now but I wanted to share this anyway.


You can add me as co-maintainer in the Fedora source repository, this should be the link: https://src.fedoraproject.org/rpms/mupen64plus/settings#usersgroups-tab

Comment 30 David Auer 2020-05-24 00:00:19 UTC
Sorry forgotten: My fas is "dreua"

Comment 31 Paul Howarth 2020-05-24 08:34:06 UTC
(In reply to David Auer from comment #29)
> We can instruct ld to allow multiple definitions, then the build works, but
> should we?
> 
> Builds, not tested: 
> https://src.fedoraproject.org/fork/dreua/rpms/mupen64plus/c/
> 046ba6166ed6e25d084043e12b157e9f13350c72?branch=fix-ld-muldefs
> 
> Maybe this fix from gentoo is better:
> https://bugs.gentoo.org/708054
> https://gitweb.gentoo.org/repo/gentoo.git/commit/
> ?id=447ad3ceef7905cc45267886632da13f6a2bc8ed

The ld option is a workaround, the gentoo approach is the proper fix.

Looks like it was fixed upstream already:
https://github.com/mupen64plus/mupen64plus-core/issues/712

Comment 32 David Auer 2020-05-24 12:33:59 UTC
On the linking issue:

Thanks Paul, I didn't find this yesterday. I absolutely agree that this is the way to go. (i.e. apply this patch downstream until it is released and disregard my linker workaround)


On the old release issue:

Would we violate hard rules by packaging Version 2.5.9 which is declared as pre-release / beta on the Github page [1] ? I generally agree that we should ship stable releases but this case may be worth an exception.

I asked upstream [2] for clarification because they seem to suggest that 2.5.9 is a release and additionally some distributions [3] are shipping it.


1: https://github.com/mupen64plus/mupen64plus-core/releases
2: https://github.com/mupen64plus/mupen64plus-core/issues/353#issuecomment-633222060
3: https://repology.org/project/mupen64plus/versions

Comment 33 Wade Berrier 2020-05-24 15:23:29 UTC
I made some progress on this:

* adapted upstream patch to fix the build (thanks for finding those) and pushed to the repository
* requested branches for f32 and f31
* added dreua to users list as admin

It'll be curious to see the upstream response on 2.5.9.

Comment 34 Fedora Update System 2020-05-31 21:55:56 UTC
FEDORA-2020-cd2b884759 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-cd2b884759

Comment 35 Fedora Update System 2020-05-31 21:55:57 UTC
FEDORA-2020-6ef57792c6 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-6ef57792c6

Comment 36 Fedora Update System 2020-06-01 03:12:47 UTC
FEDORA-2020-6ef57792c6 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-6ef57792c6 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-6ef57792c6

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 37 Fedora Update System 2020-06-01 03:25:51 UTC
FEDORA-2020-cd2b884759 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-cd2b884759 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-cd2b884759

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 38 Fedora Update System 2020-06-11 18:58:46 UTC
mupen64plus-2.5-7.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.

Comment 39 Fedora Update System 2020-06-11 22:56:16 UTC
mupen64plus-2.5-7.fc32 has been pushed to the Fedora 32 stable repository. If problems still persist, please make note of it in this bug report.


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