[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Robert changed: What|Removed |Added Resolution|--- |DEFERRED Status|NEW |CLOSED Last Closed||2022-03-09 12:57:08 --- Comment #47 from Robert --- For anyone finding this issue, there is a copr repo available here: https://copr.fedorainfracloud.org/coprs/rob72/DOSBox-X/ -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1919639 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Otto Urpelainen changed: What|Removed |Added Status|ASSIGNED|NEW Assignee|otu...@iki.fi |nob...@fedoraproject.org Flags|fedora-review? | --- Comment #46 from Otto Urpelainen --- There has been no progress with this package for a long time. Since I still see serious problems with bundled dependencies and licensing, I will drop this review on my part. Sorry, and thank you for all the effort that was put into this. Hans, fell free to continue if you wish. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #45 from Otto Urpelainen --- Blocking items are: 1. Marking all bundled dependencies as such and 2. Setting the License field and installed license file(s) to match actual source content and to comply with Licensing Guidelines -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #44 from Hans de Goede --- Robert, thank you for all your work on this. Otto, thank you for all your input on this. Do you think this is ready for me to review now (and sponsor Robert assuming the review goes well) or do you still see issues which need addressing ? -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #43 from Robert --- %caps is poorly documented in my opinion. I just thought to check how ping was packaged, since I knew it also used caps. In any case, a new release (0.83.13) is out and I just build it. You can find the updated spec and srpm file here: https://github.com/rderooy/dosbox-x-rpm F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=67020063 No real packaging changes though. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #42 from Otto Urpelainen --- Thank you for %doc and %caps changes, much better. I did not even know about %caps before this. The files that now go to /usr/share/doc/dosbox-x still go to /usr/share/dosbox-x, though. It is not serious, but worth noting here anyhow. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #41 from Robert --- I made another minor change, I replaced the setcap command with the %caps macro. You can find the updated spec and srpm file here: https://github.com/rderooy/dosbox-x-rpm F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=65836560 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #40 from Robert --- I have done a new build with the suggested %doc packaging changes. You can find the updated spec and srpm file here: https://github.com/rderooy/dosbox-x-rpm F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=65817372 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Robert changed: What|Removed |Added Flags|needinfo?(robert.de.rooy@gm | |ail.com)| --- Comment #39 from Robert --- Thanks Otto, I will in the next spin (might not be until the weekend...) remove the hardening and classify those files as doc. As to setcap, yes I know it gives that ignored error message during build, but it absolutely does do it during the RPM install. $ getcap /usr/bin/dosbox-x /usr/bin/dosbox-x cap_net_raw=ep -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Otto Urpelainen changed: What|Removed |Added Flags||needinfo?(robert.de.rooy@gm ||ail.com) --- Comment #38 from Otto Urpelainen --- Thank you for the new specfile. I went through it. Findings that I missed earlier: - You are right about hardened build being the default. I apologize, my mistake. Adding the line about hardening should not be required. I sent a pull request for the Guidelines about this topic: https://pagure.io/packaging-committee/pull-request/1064 - I think these files are actually documentation and it would be better to install them with %doc so they would go to documentation directory instead of data (assuming that there are not actually needed at runtime): /usr/share/dosbox-x/CHANGELOG /usr/share/dosbox-x/dosbox-x.reference.conf /usr/share/dosbox-x/dosbox-x.reference.full.conf - I see the following in build.log, perhaps you want to check if the capability setting script actually achieved its purpose? test -x /usr/sbin/setcap && setcap cap_net_raw=ep /builddir/build/BUILDROOT/dosbox-x-0.83.12-2.fc35.x86_64/usr/bin/dosbox-x unable to set CAP_SETFCAP effective capability: Operation not permitted make: [Makefile:906: install] Error 1 (ignored) -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #37 from Otto Urpelainen --- (In reply to Gregory PAKOSZ from comment #35) > Though it shouldn't be necessary: IANAL but WTFPL v2 is supposed to be > compatible with GPL v2 and v3. It's at least listed as such by Fedora: > https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses Hi Gregory, That is right, whereami's license is ok for Fedora, either of the given options would work actually. The problem I have with the licenses is not that dosbox-x could not be included in Fedora. It is just that the licenses should be listed correctly in specfile Licenses field, and the conditions for each license fulfilled. A very common case is that popular permissive licenses like MIT, BSD, APL2 all require distributing the original copyright and permissions notices with the source or compiled program. When a project bundles it dependencies like dosbox-x does, this results in requirement to include a lot of those notices. As an example, consider the Visual Studio Code notices file (from some old version, the current one is much longer): https://gist.github.com/dm/e5581d6c37b408c819ec Another example would be this Oracle HTTP Server docs page, fulfilling the license conditions of open source software they have used: https://docs.oracle.com/cd/B14117_01/server.101/b12255/license.htm -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #36 from Robert --- A breakage was found in the latest release version which effects DOS4GW games, and possibly others. So I included a patch, and have bumped the release to 0.83.12-2 The patch is already merged upstream. You can find the updated spec and srpm file here: https://github.com/rderooy/dosbox-x-rpm F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=65216496 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #35 from Gregory PAKOSZ --- Though it shouldn't be necessary: IANAL but WTFPL v2 is supposed to be compatible with GPL v2 and v3. It's at least listed as such by Fedora: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Gregory PAKOSZ changed: What|Removed |Added CC||gregory.pakosz+redhat@gmail ||.com --- Comment #34 from Gregory PAKOSZ --- Stumbled upon this ticket: does it help if I add a license to whereami? Best, Gregory -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #33 from Robert --- A new dosbox-x version was released, and I have just found a moment to package it up. You can find the updated spec and srpm file here: https://github.com/rderooy/dosbox-x-rpm F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=65172729 Spec file changes: - Removed patch as the issue was fixed upstream - Includes the MAN page - adds "BuildRequires: libslirp-devel" (needed for a new optional NE2000 back-end) - adds "Requires: hicolor-icon-theme" - adds "%global _hardened_build 1" I'm not sure about the last one being necessary, based on this: https://fedoraproject.org/wiki/Changes/Harden_All_Packages Regarding some games perhaps not working, this is likely because some games require special settings to get working. That or perhaps something broke in dosbox-x... Regarding the licenses, I reported it, but I don't have any update at this point. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #32 from Otto Urpelainen --- The process is regretfully long, but we are making progress, which is great. I found two more items: - Because the binary is given a capability, it must be compiled to use position independent by adding '%global _hardened_build 1'. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_pie - Directory hierarchy for the icon starting at /usr/share/icons/hicolor must be explicitly imported from the package that owns in by adding 'Requires: hicolor-icon-theme'. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership which I found unclear, so asked in the mailing list, too: https://lists.fedoraproject.org/archives/list/packag...@lists.fedoraproject.org/thread/FRVLCDRNBPWE4BSENJAQ5KOEUZVKT5R3/ In other news, I actually installed the package on F34 system and can confirm that it works. From the 4 games I tried, only Settlers 2 was unplayable due to mouse not working. So in general it works, great! Regarding licenses, it is really good that upstream is involved with getting it right for Fedora. GPL compatibility is also an absolute must if dosbox-x comes with a GPL license, so it is good that it has been asserted already — but it is not the only condition that is found in various licenses. All the conditions the author has put on their work have to be fulfilled, otherwise including the software in Fedora would be a copyright violation. I will give an example, reference for this are the Licensing Guidelines https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/ Original work from dosbox-x is licensed under GPLv2+, so the specfile license field will contain at least that. I picked one item from attachment licensecheck.txt that is listed as having something else than GPLv2+: *No copyright* Do What The Fuck You Want To Public License, Version 2 - dosbox-x-dosbox-x-v0.83.11/include/whereami.h dosbox-x-dosbox-x-v0.83.11/src/gui/whereami.c Checking those files, I find out that it is a bundled library with a copyright notice saying it is originally from https://github.com/gpakosz/whereami and is dual licensed under WTFPL and MIT licenses. So now License is at least (GPLv2+ and (WTFPL or MIT)). Then to licensing conditions. The only condition of MIT is this: "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software." This is fulfilled by adding the MIT license to the package through the %license field. Checking the Licensing Guidelines, there is a discouraged alternative path, but basically the rule is that the license has to be copied from upstream. So, upstream has to be contacted and asked to add the full license text (which can be copied from the whereami repo). On the other hand, WTFPL does not have any restrictions at all, so including the license text is not required. Still, the Licensing Guidelines have a SHOULD item about asking upstream to add it, just to remove any doubt regarding what the license really is. Probably it can be added when the MIT case is resolved without any extra effort. This procedure has to be repeated for all the content that gets compiled into or otherwise included in the rpm. Details way depending on the exact licenses. (Note that in the particular case of gpakosz/whereami, upstream could leverage the great freedom of WTFPL and simply relicense the whole thing under GPLv2+ and their own name. Complication removed. This may not be an option for other files with different licenses. In theory Fedora could also do the relicensing, the guidelines do not say anything about it, we could ask in the mailing lists — but my hunch is that the answer would be to always use the same license upstream has and fix possible hurdles upstream.) I am also not sure how the resulting combined license for dosbox-x executable should be packaged. Can multiple license files be packaged? Can the multiple licenses be concatenated with a script in the specfile? Or does Fedora only accept a single file that is maintained upstream and copied verbatim? Most probably we have to find an answer to that question before this review is complete. but before doing that, I would really like to first follow the two paths that are available for simplyfying the situation: source minification (remove clutter so it is easier to understand what content is really needed and how it is licensed) and library unbundling (breaking the monolith to multiple small and simple packages — many of which hopefully already exist in Fedora). -- 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@lists.fedoraproject.org To
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #31 from Robert --- Here is a comment on the licenses from one of the developers (Wengier): About licensing - some libraries are released under the zlib/libpng license, Apache license, BSD license, LGPL license, Expat license, etc. Most (if not all) of them appear to be compatible with either GNU GPL v2 or v3. The vs2015 directory may not need to be counted by the way. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639
Robert changed:
What|Removed |Added
Flags|needinfo?(robert.de.rooy@gm |
|ail.com)|
--- Comment #30 from Robert ---
The man page is not in the linked spec file, because it is a spec file for
0.83.11. The man page was added after, and will be part of 0.83.12, which
should be out in the next few days.
I will make the update for %{_sbindir}, and will ask the main developers to
comment on the two big items.
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #29 from Otto Urpelainen --- Created attachment 1767183 --> https://bugzilla.redhat.com/attachment.cgi?id=1767183=edit licensecheck.txt -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639
Otto Urpelainen changed:
What|Removed |Added
Flags||needinfo?(robert.de.rooy@gm
||ail.com)
--- Comment #28 from Otto Urpelainen ---
Great, the items from previous batch are now all resolved. Here are two more:
- This would be better with %{_sbindir} macro here instead of hard coded
/usr/sbin. Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros
> if [ -x /usr/sbin/setcap ]; then
- Man page should be installed. I see that you have added it upstream, is it
just accidentally missing from %install? Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages
I only have a couple of items remaining, the big two being:
1. Licenses, already discussed before. I will attach the list of licenses
fedora-review found; there is more than GPLv2+ even if vs2015 is not counted.
Could you check that and find out, what actually should be written to License
field, following
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/?
This will be easier if the source rpm minification is done first, because there
will be less clutter from unused files.
2. Bundled dependencies. To me, src/libs looks like collection of bundled
libraries. Could you check the content of src/libs against guidelines at
https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling and
comment?
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #27 from Robert --- A few answers... Regarding the patch, I will keep it in mind for the future. Yes the patch was submitted upstream and merged. For the .desktop and .metainfo.xml files, I have updated the spec file to do the validation and have submitted the changes upstream. A new DOSBox-X release is due in a few days, and I will update the SRPM then. In the meantime, I have placed an updated spec file online with the changes. https://github.com/rderooy/dosbox-x-rpm/blob/master/dosbox-x.spec -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #26 from Hans de Goede --- (In reply to Otto Urpelainen from comment #25) > I get the feeling that the Review Guidelines are outdated regarding this > topic. I agree. > Robert, do not bother with spec file translations. Ack. > I will see later if something can be done to clarify the guidelines regarding > this. Great, thank you for looking into this. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #25 from Otto Urpelainen --- (In reply to Hans de Goede from comment #24) > > - Description and summary should be available in all supported languages. I > > see some translations in the .desktop file, do those count as "supported > > languages"? Reference: Unclear where this comes from, I cannot find this is > > Packaging Guidelines, still fedora-review lists this as a SHOULD item. > > I've always interpreted this as "don't remove translations" (when using say > an upstream provided specfile as base) there is really no need to add > translations to spec-files. Esp. since now a days all the GUI tools don't > care about the spec-file Summary / description anyways. In e.g. > gnome-software the summary comes from the .desktop and/or the appdata file > and the description from the appdata. I get the feeling that the Review Guidelines are outdated regarding this topic. Robert, do not bother with spec file translations. I will see later if something can be done to clarify the guidelines regarding this. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #24 from Hans de Goede --- > - Description and summary should be available in all supported languages. I > see some translations in the .desktop file, do those count as "supported > languages"? Reference: Unclear where this comes from, I cannot find this is > Packaging Guidelines, still fedora-review lists this as a SHOULD item. I've always interpreted this as "don't remove translations" (when using say an upstream provided specfile as base) there is really no need to add translations to spec-files. Esp. since now a days all the GUI tools don't care about the spec-file Summary / description anyways. In e.g. gnome-software the summary comes from the .desktop and/or the appdata file and the description from the appdata. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Otto Urpelainen changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #23 from Otto Urpelainen --- Some findings from review. This list is not exhaustive, since I did not have time to go through everything yet, and there were also some items from fedora-review that I still have to look up (everybody is learning something here): - Patch should have link to upstream fix, or comment explaining why it is required but cannot be upstreamed. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/PatchUpstreamStatus/ > Patch:s390x-mem.patch - Customarily, changelog entries have a blank line between them (not required as such, but that is what is commonly done). Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs - For the .desktop file, either desktop-file-install (in %install) OR desktop-file-validate (in %check or %install) must be run. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_files - AppStream .metainfo.xml must be validated with appstream-util validate-relax (in %check or %install). Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/ - Description and summary should be available in all supported languages. I see some translations in the .desktop file, do those count as "supported languages"? Reference: Unclear where this comes from, I cannot find this is Packaging Guidelines, still fedora-review lists this as a SHOULD item. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #22 from Otto Urpelainen --- Note that the source stripping can be done without changes upstream. If you check the correct old commit of Calibre package [1] (incorrectly linked above, my mistake, I am sorry for that) I linked before, you will find the following scheme: 1. The spec file refers to a source tarball that is a stripped down version of the upstream tarball 2. The is a comment in spec file explaining which script needs to be run the create the stripped down tarball 3. Said script in included in the repository Similar scheme could be used here to transform upstream sources to smaller source to be used as a starting point for Fedora packaging. Of course, it would be great for Fedora if upstream provided a tarball without bundled dependencies or material specific to other OSes or build systems Fedora does not use — but there is also the method described above. I can also suggest yet another, perhaps simpler way to get started with stripping: If unnecessary files can be removed like this in the %prep section: %prep rm -rf vs2015 # more removals After that, the removed files do not exist for the rest of processing, and fedora-review is also smart enough to not consider when checking for licenses. That way, you could find a suitably simple base. Afterwards, the source stripping script described above could be developed to make the source rpm smaller and simplify the licensing situation there also. I hope this explanation makes sense. Feel free to ask further questions if anything is unclear. [1]: https://src.fedoraproject.org/rpms/calibre/tree/fea70390a1a085be6155894425c353922861bc18 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Robert changed: What|Removed |Added Flags|needinfo?(robert.de.rooy@gm | |ail.com)| --- Comment #21 from Robert --- @otu...@iki.fi Thanks for your review so far. Just to clarify. I am a occasional contributor of the DOSBox-X project, but am not one of the main contributors/developers. I was asked if I could do this review request as I am a Fedora user, and have some packaging experience. I also did the flatpak. As to the issues raised, I will relay them back to the main developers. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Otto Urpelainen changed: What|Removed |Added Assignee|nob...@fedoraproject.org|otu...@iki.fi Flags||fedora-review? ||needinfo?(robert.de.rooy@gm ||ail.com) --- Comment #20 from Otto Urpelainen --- Ok, I took this review. Unfortunately I am not able to sponsor Robert, as I am quite recent contributor myself. We will get back to you when the review is otherwise complete. @robert.de.r...@gmail.com there seems to be agreement that creating a slimmed down source tarball is a good idea. Please see what you can do. Otherwise, I will continue the review as soon as possible. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Hans de Goede changed: What|Removed |Added Flags|needinfo?(hdegoede@redhat.c | |om) | --- Comment #19 from Hans de Goede --- > @hdego...@redhat.com are you still planning to take this for full review? I > could also take over if you do not intend to complete the review. I have my hands full with upstream kernel maintenance work, so if you can take over this review that would be great, go for it. Note that Robert does need a sponsor. If you cannot sponsor, then I can take-over once you are happy with this, do a final review and then sponsor Robert. If you can sponsor yourself, that would be even better :) As for your remarks about shrinking the source-tarbal, yes that would probably be good. I also agree that it would be good to double check the license situation, although I don't expect any issues to come up, you never know. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Otto Urpelainen changed: What|Removed |Added CC||otu...@iki.fi Flags||needinfo?(hdegoede@redhat.c ||om) --- Comment #17 from Otto Urpelainen --- @hdego...@redhat.com are you still planning to take this for full review? I could also take over if you do not intend to complete the review. Regarding packaging, I think the upstream tarball is not suitable as a source entry in the specfile. Extracted, it is a > 200 MiB beast with lots of stuff that is not needed for Fedora packaging at all, at least (sub)directories called vs2015, windows and macos, probably others too. I have been told that Calibre package [1] performs similar actions, it could be used as a base for this. [1]: https://src.fedoraproject.org/rpms/calibre Licensing-wise, I doubt that the license really is GPLv2 or GPLv2+. fedora-review gave me this: Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU General Public License, Version 2", "*No copyright* GNU General Public License, Version 2", "GNU General Public License v2.0 or later", "GNU General Public License v2.0 or later [generated file]", "GNU General Public License v3.0 or later", "Expat License [generated file]", "GNU General Public License", "zlib/libpng license", "GNU Library General Public License v2 or later", "GNU Lesser General Public License v2.1 or later", "BSD 2-clause "Simplified" License", "*No copyright* Do What The Fuck Youither Want To Public License, Version 2", "Expat License", "ISC License", "BSD 3-clause "New" or "Revised" License", "*No copyright* GNU General Public License", "*No copyright* GNU General Public License v2.0 or later", "FreeType License", "Public domain", "[generated file]", "FSF Unlimited License (with Retention) [generated file]", "FSF Unlimited License [generated file]", "*No copyright* Public domain", "Libpng License", "BSD 4-clause "Original" or "Old" License", "GNU Lesser General Public License, Version 2.1", "*No copyright* GNU Library General Public License, Version 2.0", "*No copyright* GNU Lesser General Public License", "*No copyright* zlib/libpng license", "GNU Lesser General Public License v2.1 or later [obsolete FSF postal address (Temple Place)]", "the Unlicense", "GNU Library General Public License v2 or later [obsolete FSF postal address (Temple Place)]", "the Unlicense Expat License", "FreeType License [generated file]", "NTP License [generated file]", "FSF Unlimited License (with Retention) GNU General Public License, Version 2", "FSF Unlimited License (with Retention)", "FSF Unlimited License (with Retention) GNU General Public License v2.0 or later", "*No copyright* [generated file]", "zlib/libpng license Khronos License", "zlib/libpng license Expat License", "Khronos License", "*No copyright* SGI Free Software License B", "FSF Unlimited License (with Retention) GNU General Public License, Version 2 [generated file]", "BSD 4-clause "Original" or "Old" License GNU General Public License v2.0 or later", "Apache License 2.0", "NTP License", "GNU Lesser General Public License", "GNU Library General Public License, Version 2.0", "Public domain GNU Lesser General Public License v2.1 or later", "zlib/libpng license [generated file]", "zlib/libpng license Public domain", "zlib/libpng license NTP License", "*No copyright* Boost Software License 1.0", "Boost Software License 1.0", "Apple MIT License GNU Library General Public License v2 or later", "Expat License GNU Lesser General Public License v2.1 or later", "GNU General Public License, Version 3", "*No copyright* Common Public License 1.0". 3105 files have unknown license. Not all of them end up in the binary rpm, some are only found inside vs2015 directory, for example, but the situation should be checked. It also seems that some licenses that require attaching the full license text are not respected in that manner (note that this needs to be handled for source rpm even if the file is not used for binary rpm, because the source rpm is distributed by Fedora, too). Probably it would be the best to first create a smaller source tarball as suggested above, so that there would be less files and licenses to check. -- 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@lists.fedoraproject.org To unsubscribe send an email to
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #16 from Robert --- Ok, done. This includes a small patch to fix the s390x build issue. There are no ExclusiveArch or ExcludeArch statements in the spec file any longer. SPEC and SRPM: https://github.com/rderooy/dosbox-x-rpm F33 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=63196099 F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=63196166 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #15 from Hans de Goede --- Please do add the fix for the s390x build-issue as a patch for now. Otherwise you need to add an ExcludeArch to the .spec file and we have a bunch of red-tape surrounding tracking ExcludeArch-s to avoid them being abused and to allow people who care about more niche architectures to check what is not available on their arches and why. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures Adding the patch (as a downstream patch for now) will avoid all the red-tape surrounding this which really is much easier. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #14 from Robert --- We have a fix for the issue preventing building on s390x, but I'm not sure it is worth including a patch in the RPM. It should be fixed in the next monthly release. It is not like people are likely to run a DOS emulator on a mainframe. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #13 from Robert --- As mentioned previously, I have updated the packages to the newly released 0.83.11 which fixes building on some architectures. Here are the spec and srpm files https://github.com/rderooy/dosbox-x-rpm F33 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=62899097 F34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=62899544 Unfortunately a new issue is preventing it from building on s390x, so I had to exclude it. I will try to get this fixed upstream and have already opened an issue for it on the dosbox-x gitlab. I would like to request another review at this point. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #12 from Hans de Goede --- (In reply to Robert from comment #11) > I have not had time to provide updates. But I solved the issue of DOSBox-X > not building on some architectures upstream. > With this, I can build the RPM on koji for i686, x86_64, arm, aarch64, ppc > and s390x. > > A new version of DOSBox-X with this patch included should be out on or > around the 1st of March, and I will send an updated review request then. Sounds good, thank you. I'll try to make some time to do a full review when you've posted the new version. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #11 from Robert --- I have not had time to provide updates. But I solved the issue of DOSBox-X not building on some architectures upstream. With this, I can build the RPM on koji for i686, x86_64, arm, aarch64, ppc and s390x. A new version of DOSBox-X with this patch included should be out on or around the 1st of March, and I will send an updated review request then. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #10 from Robert --- In addition to the SPEC and SRPM files linked above, here are the koji builds: F33: https://koji.fedoraproject.org/koji/taskinfo?taskID=61053742 F34: https://koji.fedoraproject.org/koji/taskinfo?taskID=61054003 Note: I had to disable building for 32bit ARM for now, I have created a bug report for this. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #9 from Robert --- DOSBox-X 0.83.10 was just released, and I have updated the spec and srpm. It now uses the %configure macro. SPEC file: https://github.com/rderooy/dosbox-x-rpm/blob/master/dosbox-x.spec SRPM file: https://github.com/rderooy/dosbox-x-rpm/raw/master/dosbox-x-0.83.10-1.fc33.src.rpm I don't have any koji builds right, but will get some done later today when I'm back home. The new version is also already up on flathub: https://flathub.org/apps/details/com.dosbox_x.DOSBox-X -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #8 from Robert --- Thanks for your review so far. As regards the format-security error, that issue had apparently already been fixed upstream. Since a new release (0.83.10) should be out in the next day, I will wait until it is released, and update the spec file. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #7 from Vladislav Kazakov --- > Also, I used [AT] instead of @ in the email to reduce spam, I thought that > was acceptable. Yes, this is acceptable. I just thought it was a copying error. >>The moment I change that, it seems that the configure flags specified are >>thrown out of the window and the compile just fails as it ends up looking for >>SDL1 libraries instead of SDL2. > If someone can tell me how to use the %configure macro while keeping the > flags, I would appreciate it. You can keep flags just like that: """ %configure --enable-core-inline --enable-debug=heavy --enable-sdl2 """ But it falls anyway whille compiling sdlmain.cpp, because the %configure adds -Werror=format-security to g++: """ sdl_ttf.c:334:18: error: format not a string literal and no format arguments [-Werror=format-security] 334 | TTF_SetError(msg); | ^ """ It would be nice to fix this in upstream. Or you can just remove -Werror=format-security. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #6 from Robert --- A note about the GPL version. I changed it to GPL2+, since I thought I made a mistake. But looking at the DOSBox COPYING file, it seems to be GPL2 as the heading does not contain the "any later version" text. https://sourceforge.net/p/dosbox/code-0/HEAD/tree/dosbox/trunk/COPYING DOSBox-X use the same file. https://github.com/joncampbell123/dosbox-x/blob/master/COPYING -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #5 from Robert --- I think I have fixed all the issues mentioned, apart from changing ./configure to %configure The moment I change that, it seems that the configure flags specified are thrown out of the window and the compile just fails as it ends up looking for SDL1 libraries instead of SDL2. If someone can tell me how to use the %configure macro while keeping the flags, I would appreciate it. Also, I used [AT] instead of @ in the email to reduce spam, I thought that was acceptable. spec file: https://github.com/rderooy/dosbox-x-rpm/blob/master/dosbox-x.spec srpm file: https://github.com/rderooy/dosbox-x-rpm/raw/master/dosbox-x-0.83.9-1.fc33.src.rpm f33 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=60490574 f34 build: https://koji.fedoraproject.org/koji/taskinfo?taskID=60490134 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639
--- Comment #4 from Hans de Goede ---
Some more remarks:
"License: GPLv2"
The orginal dosbox is GPLv2 or later, which we write as GPLv2+, is this not the
case for dosbox-x?
Please drop all the Requires: on libraries rpm will auto generate these based
on the used sonames. Note you will want to keep the fluid-soundfont-gm
Requires.
"""
desktop-file-install \
--dir=%{buildroot}%{_datadir}/applications \
--set-icon=com.dosbox_x.DOSBox-X \
contrib/linux/com.dosbox_x.DOSBox-X.desktop
install -p -m 0644 contrib/icons/dosbox-x.svg
%{buildroot}%{_datadir}/icons/hicolor/scalable/apps/com.dosbox_x.DOSBox-X.svg
%exclude %{_datadir}/icons/hicolor/scalable/apps/dosbox-x.svg
"""
Maybe just have the .desktop file used the already installed dosbox-x.svg ?
"install -p -m 0644 -Dt %{buildroot}%{_datadir}/%{name}/glshaders
contrib/glshaders/*.glsl"
Why ? Please add a comment explaining why this is done
Please verify the installed appdata file:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 --- Comment #3 from Hans de Goede --- Robert, Please create a new version addressing Vladislav's requests. For the next version please upload a srpm and spec file somewhere and provide links/URLs to the spec-file and srpm. We must have a srpm for review so that we can verify that the sources tarbal which you will upload into the look-aside cache when importing the package matches the upstream tarbal. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1919639] Review Request: DOSBox-X - DOS/x86 emulator
https://bugzilla.redhat.com/show_bug.cgi?id=1919639 Hans de Goede changed: What|Removed |Added CC||hdego...@redhat.com Doc Type|--- |If docs needed, set a value --- Comment #2 from Hans de Goede --- (In reply to Vladislav Kazakov from comment #1) > Note that make can be omitted. That is no longer true, starting with Fedora 34 make will not be part of the default package set in the buildroot. So it needs to have an explicit BuildRequires, so please keep a "BuildRequires: make" for the next version. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
