Bug 1264546
Summary: | Review Request: soletta - A framework for making IoT devices | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gustavo Lima Chaves <gustavo.lima.chaves> | ||||
Component: | Package Review | Assignee: | Paulo Andrade <paulo.cesar.pereira.de.andrade> | ||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | unspecified | ||||||
Version: | rawhide | CC: | fidencio, i, package-review, paulo.cesar.pereira.de.andrade, sergio | ||||
Target Milestone: | --- | Flags: | paulo.cesar.pereira.de.andrade:
fedora-review+
|
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
URL: | https://github.com/solettaproject/soletta/ | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2016-09-13 18:08:11 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Gustavo Lima Chaves
2015-09-18 18:42:59 UTC
I can't sponsor you, but I can give some suggestions. 1. Please include your email in changelog. You can try rpmdev-bumpspec to see what exactly is changelog, obviously the current one is poor. 2. "%define soletta_major 0 %define soletta_minor 0 %define soletta_build 1 %define soletta_release beta5" https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define 3. Drop Group tags in all packages. 4. It's better to put %post(un/trans) after %install/%check but before %files. 5. "This package contains the sysctl linux-micro module for %{name}. The module sets kernel parameters from sysctl.conf files. This service will mimic systemd-sysctl.service and read the settings from '/etc/sysctl.conf' or '/run/sysctl.d', '/etc/sysctl.d', '/usr/local/lib/sysctl.d', '/usr/lib/sysctl.d', '/lib/sysctl.d'. Files are processed in alphabetical order. See http://www.freedesktop.org/software/systemd/man/systemd-sysctl.service.html. " Sorry, we don't use some of these paths. Please remove them. 6. The build log is silent, I could only see GEN * CC * LD * BIN * This makes it impossible to detect if it's been built correctly: https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags You must compile packages with %optflags and %__global_ldflags for linking, note these need to be inserted, but not substituted of all flags since you might use some custom flags as well. 7. Drop %defattr(-, root, root, -) 8. %{_includedir}/soletta/* No, you forgot %{_includedir}/soletta/ itself, this will only include files underneath the dir without dir itself. 9. Same as above, %{_datadir}/soletta and %{_libdir}/soletta weren't included, but since different subpackages put files inside, you need to decide on your own. 10. You use %license, but put it in lib%{name}-pin-mux-module-Edison only, that's wrong, you should put it into lib%{name}. (because every subpkg depends on it based on the spec from my view) 11. I don't see any packages with name %{name}, so isn't it better to rename this spec to libsoletta? 12 Last question, do we really need such many subpackages? Christopher, thanks for jump in the review, I hope you don't mind to answer a few questions. Why "3. Drop Group tags in all packages"? They are not the same, better have it specified, no? Gustavo, I can be your sponsor for sure. I would like to ask you to follow the suggestions given by Christopher. (In reply to Fabiano Fidêncio from comment #2) > Christopher, thanks for jump in the review, I hope you don't mind to answer > a few questions. > > Why "3. Drop Group tags in all packages"? They are not the same, better have > it specified, no? > They are optional on Fedora and not required, only for Epel (not sure for which version), see: https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag : - All current versions of Fedora (and their respective RPM versions) treat the Group tag as optional. Packages may include a Group: field for compatibility with EPEL, but are not required to do so. And they litter the spec-file just more. It's hard to read anyway, because of the amount of sub-packages. By the way: Gustavo please fix the url's for Source0 and Source1, they do not work. What's the cause for keeping the "config"-file locally ? Thank you guys. I'll be fixing the pointed issues one by one today. Later on I'll resubmit, with any possible doubts posted here. > I can't sponsor you, but I can give some suggestions. Thanks a lot for the suggestions, Christopher! I guess Fidencio and I will continue fine with this (thank you too, Panda!). > > 1. Please include your email in changelog. Done. I really wish I could have a single, initial changelog for an initial packaging attempt. Is that possible? > > You can try rpmdev-bumpspec to see what exactly is changelog, obviously the > current one is poor. > > 2. "%define soletta_major 0 > %define soletta_minor 0 > %define soletta_build 1 > %define soletta_release beta5" > > https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define Thanks a lot, I won't forget. > > 3. Drop Group tags in all packages. Okay, better for me. > > 4. It's better to put %post(un/trans) after %install/%check but before %files. Done. > > 5. "This package contains the sysctl linux-micro module for %{name}. The > module sets kernel parameters from sysctl.conf files. This service > will mimic systemd-sysctl.service and read the settings from > '/etc/sysctl.conf' or '/run/sysctl.d', '/etc/sysctl.d', > '/usr/local/lib/sysctl.d', '/usr/lib/sysctl.d', '/lib/sysctl.d'. Files > are processed in alphabetical order. See > http://www.freedesktop.org/software/systemd/man/systemd-sysctl.service.html. > " > > Sorry, we don't use some of these paths. Please remove them. All linux-micro modules were moved to the base lib package, so these texts are no more. I did that because linux-micro would eventually vanish in a f23 version of the spec. > > 6. The build log is silent, I could only see > > GEN * > CC * > LD * > BIN * Well, by default our build log IS indeed silent. You can see all that's happening with V=1, if you wish. > > This makes it impossible to detect if it's been built correctly: > > https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags > > You must compile packages with %optflags and %__global_ldflags for linking, > note these need to be inserted, but not substituted of all flags since you > might use some custom flags as well. Nevertheless, I added those, thanks. The only "strange" output you'll see there are cpio errors when trying to generate debug symbols for the duktape (JavaScript) sub-module. There's an open ticket for the lib authors to remove #line directives from their released code, that should vanish soon (next release, maybe). > > 7. Drop %defattr(-, root, root, -) Done. > > 8. %{_includedir}/soletta/* > > No, you forgot %{_includedir}/soletta/ itself, this will only include files > underneath the dir without dir itself. Ok, various %dir added. > > 9. Same as above, %{_datadir}/soletta and %{_libdir}/soletta weren't included, > but since different subpackages put files inside, you need to decide on your > own. Ditto. > > 10. You use %license, but put it in lib%{name}-pin-mux-module-Edison only, > that's wrong, you should put it into lib%{name}. (because every subpkg depends > on it based on the spec from my view) Now it happens just after the main lib's files. > > 11. I don't see any packages with name %{name}, so isn't it better to rename > this spec to libsoletta? I intend to package other things on the soletta project umbrella here. Namely https://github.com/solettaproject/soletta-dev-app is the next candidate, so... > > 12 Last question, do we really need such many subpackages? 11 of them were merged with the main lib now. The others I'd like to keep -- they are optional for most use cases and may require different deps. Again, thanks a lot for the review. I'll have this new spec uploaded soon for you to see. > By the way: > Gustavo please fix the url's for Source0 and Source1, they do not work. Sorry, indeed the 0 one was wrong. What's the point of github having the downloaded file != the URL file? :/ > What's the cause for keeping the "config"-file locally ? What's the rationale for this? We distribute all deps on external servers? This is only the configuration for Fedora systems, I thought I could skip having to address this specifically in a URL. So, after this 1st round of reviews, here we are: New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v3/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v3/soletta-0.0.1-beta5.fc22.src.rpm Successful koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=11182749 Successful copr build: https://copr.fedoraproject.org/coprs/glchaves/soletta/build/117590/ Please let me know if I forgot anything from the reviews. > Sorry, indeed the 0 one was wrong. What's the point of github having the > downloaded file != the URL file? :/ "spectool -g x.spec" downloads the sources. Most reviewers also use "fedora-review -b 1264546" for reviews, this also checks the source urls. fedora-review fails to build, could you please run fedora-review yourself first and fix the issues. There are also guidelines on the Source, please see this section for the source packaging guidelines: https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Hosting_Services I don't fully understand the release of the package. Release: tag should specifies the release of the package, eg. you update the specfile and should increment the release by one. It seems to me that you filled in upstreams release, which should be part of version. The beta5 can also be problematic as this is not numerical and rpm won't be able to compare with newer version. Please see guidelines: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages Thanks a lot, Piotr, I'll be looking at the issues you pointed today. BTW, since Fidencio can't be my sponsor (he seems not to classify for that), would you? :) I'm sorry but am not a sponsor either. Best way to get sponsored fast is to do a lot of informal package reviews so you will show you understand the package guidelines. Then link to the reviews from this bug. Potential sponsors will notice the reviews. Other option is to find a packager willing to maintain this package, you could then become co-maintainer without going through the sponsoring process. Thanks, Piotr. > fedora-review fails to build, could you please run fedora-review yourself first > and fix the issues. I builds fine here. Would it be that I'm under f22 and you're running a > version? Note that I target f22 in this bug. I'll eventually put conditionals for exchanging Soletta's config file for fedoras >= 23. Let's see if you or someone can help with this fedora-review tool's output, though (just manual stuff, I got no ! entries). [ is this true or moot? ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/soletta/flow/descriptions, /usr/share/soletta/flow Now rpmlint: libsoletta.x86_64: E: non-standard-executable-perm /usr/lib64/soletta/modules/linux-micro/network-up.so 775 I'm sure that under a virtual machine I get this file listed as 755 after installation. Is that any difference of umask during the tool's processing or what? > The beta5 can also be problematic as this is not numerical and rpm won't be able to compare with newer version. Please see guidelines: Mm, thanks. I'm now with the following schema, as read on the guidelines: %global soletta_major 0 %global soletta_minor 0 %global soletta_build 1 %global soletta_tag beta5 %global soletta_duktape_tag beta2 Summary: A framework for making IoT devices Name: soletta Version: %{soletta_major}.%{soletta_minor}.%{soletta_build} Release: 0.1.%{soletta_tag}%{?dist} > There are also guidelines on the Source, please see this section for the source packaging guidelines: Fixed, thanks a lot. You are supposed to test-build against Rawhide and review for Rawhide. > [ is this true or moot? ]: https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging:UnownedDirectories The fedora-review tool is pretty good at finding so-called "unowned directories". If you want to verify such issues, run "rpmls … | grep ^d" on the built package and check that every directory is included. As the next step, review the subpackage dependency chain and verify that the packages providing those directory entries are pulled in. Skimming over the spec file, it seems you have the directory ownership for various dirs in the -devel package, but the many subpackages don't depend on the -devel package. Based on that I suggest you move the directory ownership or create a -common or -filesystem subpackage (as per the linked guidelines, see the yellow box). Version lgtm now. See my build logs here: https://gist.github.com/piotr1212/a8ed3ffc34bba9c88794 Package reviews should be against rawhide (f24 currently). Please check if your mock config points to rawhide: ➜ ls -l /etc/mock/default.cfg lrwxrwxrwx. 1 root root 35 Aug 28 21:18 /etc/mock/default.cfg -> /etc/mock/fedora-rawhide-x86_64.cfg warning: bogus date in %changelog: Tue Sep 2 2015 Gustavo Lima Chaves <gustavo.lima.chaves> - 0.0.1-beta5 This one should be easy to fix. Just add an space between "Sep" and "2", or change it to "Sep 02" if you prefer that notation. Thanks, guys. I'm working on the spec to test it against rawhide too. It should not take long to post the update stuff here. > warning: bogus date in %changelog > Just add an space between "Sep" and "2", or change it to "Sep 02" if > you prefer that notation. That's *not* what the warning is about. "Tue Sep 2" is bogus, because either it's "Wed Sep 2" or "Tue Sep 1". Watch this: $ date -d 'Tue Sep 2 2015' Wed Sep 2 00:00:00 CEST 2015 glchaves's scratch build of soletta-0.0.1-0.1.beta5.fc22.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11215731 > That's *not* what the warning is about. "Tue Sep 2" is bogus, because either it's "Wed Sep 2" or "Tue Sep 1". Watch this:
Yes, Michael, I knew that and had a local fix already. BTW, I suppose it's OK to have the 1st spec in with just an initial changelog entry, right?
glchaves's scratch build of soletta-0.0.1-0.1.beta5.fc22.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11215899 After second round of reviews, here we are: New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v4/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v4/soletta-0.0.1-0.1.beta5.fc22.src.rpm Successful koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=11215899 Successful copr build: https://copr.fedoraproject.org/coprs/glchaves/soletta/build/118188/ BTW, would you sponsor me, Michael? Thanks for the tips so far :) We released beta6 now, it should not take long till I update the specs accordingly. We're approaching the actual first release, too. glchaves's scratch build of soletta-0.0.1-0.1.beta7.fc22.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11478871 glchaves's scratch build of soletta-0.0.1-0.1.beta7.fc22.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11479077 The second build just above failed because it now include make checks and it fails on i686. I'll fix that and re-submit. Hi Gustavo, We talked briefly in Latinoware, and I understand you want this package in Fedora :) And I am sure it would be very useful. I do not know if you want to package other software, otherwise, an easier plan would be to approach a packager, that would package it for you. So, at first I would like to know if you only plan to package solleta for Fedora :) I believe the package is in good shape, but I would like a reply for the issues below. ---8<--- I suggest just using solleta for the package, no need for lib%{name}, unless you can give a good reason for that, in which case, should rename the package to libsolleta. For example, instead of %package -n lib%{name}-flow-module-accelerometer have %package flow-module-accelerometer ---8<--- Please put version information in the Version tag, not in the Release tag, and bump Release every time you make an update. For example, instead of Version: %{soletta_major}.%{soletta_minor}.%{soletta_build} Release: 0.1.%{soletta_tag}%{?dist} have Version: %{soletta_major}.%{soletta_minor}.%{soletta_build}.%{soletta_tag} Release: 1%{?dist} ---8<--- The license should be "BSD and MIT". And I am afraid probably actually be "BSD and MIT and GPLv2+", due to files under tools/kconfig/lxdialog The files are not part of runtime, but are in the tarball. duktape files are under MIT license. ---8<--- Please explain what did you mean with the line %{?%{name}_debug_package} in the spec file. ---8<--- Also please explain a bit about linux-micro in fedora < 23, and systemd in newer fedora. Did you test it in newer Fedora? I suppose you tested the package, interacting with real hardware on f22 :) ---8<--- You do not need to write %dir %{_includedir}/soletta %{_includedir}/soletta/* Just write %{_includedir}/soletta/ The ending / will make it clear is is a directory (and contents). ---8<--- Kind of related to above, you should move ownership of base directories to a package required by all subpackages, for example, instead of having in -devel this: %dir %{_datadir}/soletta/flow/descriptions ... %dir %{_datadir}/soletta/flow/descriptions/... make the main package owner of the directory, with: %dir %{_datadir}/soletta/flow/descriptions ---8<--- From reading above, you should be working on it, but I see there are several test subdirectories, but the latest spec does not have a %check section. You should run the tests, and give explanations (can be just a comment in the spec) where it is expected to fail. ---8<--- You should fix the wrong permissions. Better to find out form where they are coming. Likely from CP="cp -p". In the worst case, just run something like find %{buildroot} -perm 0775 | xargs chmod 0755 in %install > Hi Gustavo, Hi, Paulo! > We talked briefly in Latinoware, and I understand you want > this package in Fedora :) And I am sure it would be very > useful. I do not know if you want to package other software, > otherwise, an easier plan would be to approach a packager, > that would package it for you. So, at first I would like > to know if you only plan to package solleta for Fedora :) > > I believe the package is in good shape, but I would like > a reply for the issues below. We want to start packaging the soletta framework (library, devel headers and tools), but soon we'd also like to package another part of the framework -- Soletta Dev App (https://github.com/solettaproject/soletta-dev-app). That's a web-based environment that sits on the target board and exposes a web page where a developer can interact with soletta in a visual way to test his/her flow-based programming programs. No hurry, though, I'll start and get it right with Soletta solely first. It would be cool, though, to have it all in the same spec. Is that your concern, we'd have to start with all in there right away? > > > ---8<--- > I suggest just using solleta for the package, no need for lib%{name}, > unless you can give a good reason for that, in which case, should > rename the package to libsolleta. For example, instead of > > %package -n lib%{name}-flow-module-accelerometer > > have > > %package flow-module-accelerometer That's already done locally here, as I got the same feedback from some folks at IRC. > > > ---8<--- > Please put version information in the Version tag, not in the > Release tag, and bump Release every time you make an update. > For example, instead of > > Version: %{soletta_major}.%{soletta_minor}.%{soletta_build} > Release: 0.1.%{soletta_tag}%{?dist} > > have > > Version: %{soletta_major}.%{soletta_minor}.%{soletta_build}.%{soletta_tag} > Release: 1%{?dist} Okay. Do we make updates and changelog bumps here in process of package review too (before any publishment)? That's a doubt I had before and no formal answer came yet. > > ---8<--- > The license should be "BSD and MIT". And I am afraid probably actually > be "BSD and MIT and GPLv2+", due to files under tools/kconfig/lxdialog > The files are not part of runtime, but are in the tarball. duktape > files are under MIT license. Fair, will do. > > > ---8<--- > Please explain what did you mean with the line > %{?%{name}_debug_package} > in the spec file. I guess I can remove that, as the -debug package is built anyway (I just wanted to make sure it's generated). > > > ---8<--- > Also please explain a bit about linux-micro in fedora < 23, and > systemd in newer fedora. Did you test it in newer Fedora? I suppose > you tested the package, interacting with real hardware on f22 :) > I did test it reasonably on f22 and I'll start testing it harder on f23, don't worry. To explain the linux-micro saga, I'll translate what I said to Hélio (hcastro) at #soletta: Soletta has a platform module with 2 possible backends: systemd on linux-micro (the latter being targeted to smaller linux systems with no systemd, in which case it tries to emulate some of the things systemd would provide). The thing is we only detect we're on a systemd-capable system if systemd's version is at least 221 (I think we did that for sd-bus API usage we rely on). Thus, that decision -- when packaging for f23+, we put systemd as our platform, otherwise, we get linux-micro. Now, linux-micro is separated in modules, too, because a user might want not to have all the platform functionality we provide. That's why it has a couple of packages just for it, as well. > > ---8<--- > You do not need to write > > %dir %{_includedir}/soletta > %{_includedir}/soletta/* > > Just write > > %{_includedir}/soletta/ > > The ending / will make it clear is is a directory (and contents). Fair, thanks. > > > ---8<--- > Kind of related to above, you should move ownership of base > directories to a package required by all subpackages, for example, > instead of having in -devel this: > > %dir %{_datadir}/soletta/flow/descriptions > ... > %dir %{_datadir}/soletta/flow/descriptions/... > > make the main package owner of the directory, with: > > %dir %{_datadir}/soletta/flow/descriptions > Okay. > > ---8<--- > From reading above, you should be working on it, but I see > there are several test subdirectories, but the latest spec > does not have a %check section. You should run the tests, and > give explanations (can be just a comment in the spec) where it > is expected to fail. What I mentioned was WIP, that's why it's not published here yet. I decided to answer you first, but soon I'll land the updated SRPM and stuff, right? > > > ---8<--- > You should fix the wrong permissions. Better to find out form > where they are coming. Likely from CP="cp -p". In the worst > case, just run something like > find %{buildroot} -perm 0775 | xargs chmod 0755 > in %install Mm, okay. Can I ask which of the tools gave you the output so I can trace it here too? I'll exercize all of them, just in case. Thanks a lot for you support! > Do we make updates and changelog bumps here in process of > package review too (before any publishment)? Yes. It's only pointed out at https://fedoraproject.org/wiki/FrequentlyMadePackagingMistakes because for the review process it has been assumed that modifying a src.rpm implies changing the spec file and publishing the changes as a _new_ src.rpm release. Occasionally, during review somebody skips the Release bump, though, and that makes reviewing less convenient (with the tools from "rpm -ql rpmdevtools|grep diff" in mind, for example). > %{_includedir}/soletta/ > > The ending / will make it clear is is a directory (and contents). As explained in the guidelines, the trailing slash is purely cosmetic. The lines %{_includedir}/soletta and %{_includedir}/soletta/ achieve the same, regardless of whether "soletta" is a file or a directory tree. The trailing slash only increases readability of the spec file, making it explicit that the %files line doesn't refer to a single file. > Yes. It's only pointed out at > > https://fedoraproject.org/wiki/FrequentlyMadePackagingMistakes > > because for the review process it has been assumed that modifying a src.rpm implies changing the spec file and publishing the changes as a _new_ src.rpm release. > > Occasionally, during review somebody skips the Release bump, though, and that makes reviewing less convenient (with the tools from "rpm -ql rpmdevtools|grep diff" in mind, for example). Thanks a lot, Michael! Sorry for missing this FAQ. Now, with the version string hopefully done right, I'll also do a Release bump. glchaves's scratch build of soletta-0.0.1.beta8-1.fc22.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11518781 Hi Gustavo, My concern when asking if you plan to package other software is that sometimes, getting sponsored may not be a so easy task :) and sometimes one could have just one package in Fedora, and forget about it. People usually starts with a smaller package, and participate a lot on informal reviews, to show knowledge of the work flow and how to create good quality rpm packages; what an upstream developer may not have enough time to do. About the permissions issue, I did run: $ fedora-review -b 1264546 on this setup: $ ll /etc/mock/default.cfg lrwxrwxrwx 1 root root 25 Oct 4 2014 /etc/mock/default.cfg -> fedora-rawhide-x86_64.cfg and my computer is also running rawhide (as of 2 weeks ago). You also saw it in #c12 """ libsoletta.x86_64: E: non-standard-executable-perm /usr/lib64/soletta/modules/linux-micro/network-up.so 775 """ I did a quick search, looks like mock uses an umask to group writable files since https://git.fedorahosted.org/cgit/mock.git/commit/?id=6533774ebda67addfb6e701246f3a96c5e62bcd9 The install command by default does not give group write permission, while "cp -p" copies it exactly. Once checking the next srpm, with %check enabled, subpackages renamed, etc, I can assign this bug to myself, and do at least one more review. Unless someone else wants to be your sponsor :) > Hi Gustavo, Hi, Paulo. > > My concern when asking if you plan to package other software is > that sometimes, getting sponsored may not be a so easy task :) > and sometimes one could have just one package in Fedora, and forget > about it. > People usually starts with a smaller package, and participate > a lot on informal reviews, to show knowledge of the work flow > and how to create good quality rpm packages; what an upstream > developer may not have enough time to do. I indeed have a lot of my time spent on development, but my intention is to be responsible for the package at all times, naturally, coming in pronto to fix any issue the arise and always doing updates. I can try better to help on reviewing other packages too, but being a full time packager is not possible for me :(. Is that a blocker? > > > > About the permissions issue, I did run: > > $ fedora-review -b 1264546 > > on this setup: > > $ ll /etc/mock/default.cfg > lrwxrwxrwx 1 root root 25 Oct 4 2014 /etc/mock/default.cfg -> fedora-rawhide-x86_64.cfg > > and my computer is also running rawhide (as of 2 weeks ago). > You also saw it in #c12 > """ > libsoletta.x86_64: E: non-standard-executable-perm /usr/lib64/soletta/modules/linux-micro/network-up.so 775 > """ > I did a quick search, looks like mock uses an umask to group > writable files since > https://git.fedorahosted.org/cgit/mock.git/commit/?id=6533774ebda67addfb6e701246f3a96c5e62bcd9 > The install command by default does not give group write > permission, while "cp -p" copies it exactly. I think the issue is now gone with the removal of the "cp -p" part. > > > > Once checking the next srpm, with %check enabled, subpackages > renamed, etc, I can assign this bug to myself, and do at least > one more review. > Unless someone else wants to be your sponsor :) Nice! I hope you did not step back on the idea of sponsoring us :) Now for the status update. We're now o beta8, where I fixed so check issues we were having. The thing is I could not advance we would have issues in the arm build (now getting an arm VM to investigate better). Also, copr's kernel does not have crypto_user support, what we used in the tests, too. beta9 tag was rolled for that -- openssl backend is now the default for message digest in Soletta. Besides the issues on the arm build, here go what we got now, so at least the rest of the SRPM is reviewed: New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v5/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v5/soletta-0.0.1.beta8-1.fc22.src.rpm koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=11518781 I wish to keep, for now, the sole changelog line (before we get this right), and because the version changed, the release is still 1. I'll add an attachment with the diff from v4 to v5 of the spec. Created attachment 1084917 [details]
v4 to v5 differences of the spec
This is to aid in the changes from v4 to v5. If this is not needed, please let me know, for future changes.
Hi Gustavo, I started the process to become your sponsor. I believe you showed that you will have enough persistence to maintain the package, and add new packages to Fedora :) You should update the changelog as if the package were already in Fedora. I will review the new package, and let you know if anything else is required before the first official Fedora package :) The first issue I see in a quick look is that you did %package %{name}-subpackage it should have been %package subpackage now you have packages named: soletta-soletta-subpackage When not using -n, "%{name}-" is prepended to the package name. Hi Gustavo, ---%<--- First we will need to sort out some issues with the third party software. I see there is a review request for tinycbor at https://bugzilla.redhat.com/show_bug.cgi?id=1269001 It is the same thing right? About duktape I do not see any review or existing package. Could tinycbor and/or duktape be used as external dependencies? If not, what is the reason? Note that Fedora is in the process of having more relaxed rules for bundled packages, but it should only be used if there is no other option. ---%<--- Another issue I noticed, is that %install appears to redo all compilations, after regenerating header files. Some issue with make rules? ---%<--- There are some duplicated files. I think you misunderstood some details of adding a trailing "/". If listing only the directory, must do it as: %dir %{_libdir}/soletta/modules/ otherwise, there will be duplicates, because when listing a directory it understands the directory and its contents, for example: %{_libdir}/soletta/modules/ %{_libdir}/soletta/modules/flow/ the line %{_libdir}/soletta/modules/ already includes %{_libdir}/soletta/modules/flow/ Another issue is the line: %{_includedir}/soletta/* it should have been: %{_includedir}/soletta/ otherwise, the directory %{_includedir}/soletta/ will not have a owner package. ---8<--- Please comment about the warning: In function '__fread_alias', inlined from 'sol_memmap_write_raw_do.isra.2' at ./src/lib/io/sol-memmap-storage.c:169:15: /usr/include/bits/stdio2.h:290:9: warning: call to '__fread_chk_warn' declared with attribute warning: fread called with bigger size * nmemb than length of destination buffer return __fread_chk (__ptr, __bos0 (__ptr), __size, __n, __stream); It is caused by the fread in this code: if (mask) { uint64_t value = 0, old_value; uint32_t i, j; for (i = 0, j = 0; i < entry->size; i++, j += 8) value |= (uint64_t)((uint8_t *)buffer->data)[i] << j; ret = fread(&old_value, entry->size, 1, file); It looks suspicious. Shouldn't old_value be declared as size_t, and instead of entry->size, write sizeof(old_value) or sizeof(size_t) ? If the size is guaranteed to fit, it will have issues in big endian architectures. ---8<--- Also fix the %package %{name}- issue. ---8<--- For the next review, please also update the changelog. This also serves as a way to follow the history of the package :) > Hi Gustavo, Hi, Paulo. > > I started the process to become your sponsor. > > I believe you showed that you will have enough persistence > to maintain the package, and add new packages to Fedora :) Thank you so much! Yes, I'll do my best at all times, don't worry. > > You should update the changelog as if the package were already > in Fedora. Ok, from now on, I'll do that. > > I will review the new package, and let you know if anything > else is required before the first official Fedora package :) > > The first issue I see in a quick look is that you did > > %package %{name}-subpackage > > it should have been > > %package subpackage > > now you have packages named: > > soletta-soletta-subpackage > > When not using -n, "%{name}-" is prepended to the package > name. Mmm, fine. I'll fix it. > > Hi Gustavo, > > ---%<--- > First we will need to sort out some issues with the third party > software. > > I see there is a review request for tinycbor at > https://bugzilla.redhat.com/show_bug.cgi?id=1269001 > It is the same thing right? Yes, it is. As soon as they get in, we can change to depend and link to it. > > About duktape I do not see any review or existing package. > > Could tinycbor and/or duktape be used as external dependencies? > If not, what is the reason? Duktape specifically can't, as they only distribute the sources at will, to be bundled by user projects. > > Note that Fedora is in the process of having more relaxed rules > for bundled packages, but it should only be used if there is no > other option. > > > ---%<--- > Another issue I noticed, is that %install appears to redo all > compilations, after regenerating header files. Some issue with > make rules? Yes, some issue on our side, it's on my TODO-list. > > > ---%<--- > There are some duplicated files. I think you misunderstood some > details of adding a trailing "/". If listing only the directory, > must do it as: > > %dir %{_libdir}/soletta/modules/ > > otherwise, there will be duplicates, because when listing a > directory it understands the directory and its contents, for > example: > > %{_libdir}/soletta/modules/ > %{_libdir}/soletta/modules/flow/ > > the line %{_libdir}/soletta/modules/ already includes > %{_libdir}/soletta/modules/flow/ > > Another issue is the line: > > %{_includedir}/soletta/* > > it should have been: > > %{_includedir}/soletta/ > > otherwise, the directory %{_includedir}/soletta/ will not > have a owner package. Sorry for the mistakes, I'll have those fixed. > > > ---8<--- > Please comment about the warning: > > In function '__fread_alias', > inlined from 'sol_memmap_write_raw_do.isra.2' at ./src/lib/io/sol-memmap-storage.c:169:15: > /usr/include/bits/stdio2.h:290:9: warning: call to '__fread_chk_warn' declared with attribute warning: fread called with bigger size * nmemb than length of destination buffer > return __fread_chk (__ptr, __bos0 (__ptr), __size, __n, __stream); > > It is caused by the fread in this code: > if (mask) { > uint64_t value = 0, old_value; > uint32_t i, j; > > for (i = 0, j = 0; i < entry->size; i++, j += 8) > value |= (uint64_t)((uint8_t *)buffer->data)[i] << j; > > ret = fread(&old_value, entry->size, 1, file); > > It looks suspicious. Shouldn't old_value be declared > as size_t, and instead of entry->size, write sizeof(old_value) > or sizeof(size_t) ? If the size is guaranteed to fit, it will > have issues in big endian architectures. I saw it too, it won't be there anymore on the next beta release. Thanks. > > > ---8<--- > Also fix the %package %{name}- issue. > > Will do. > ---8<--- > For the next review, please also update the changelog. This also > serves as a way to follow the history of the package :) Okay. It may take me a few time for the next update, since I want to sort that arm bot error first. I guess I'll try it on a raspberry-pi fedora image here, unless I can get access to some of the community arm machines :/ Thanks a lot for the review and sponsoring again :) Hi Gustavo, In the next package you submit for review, please add this to the main package: Provides: bundled(tinycbor) = 0.2 Provides: duktape-static = 1.2.2 So that it can be tracked with a query of the packages. The bundled(tinycbor) is to remember it has a bundled library that may be updated. The duktape-static is due to the policy of when a package is composed only of header files, or in this case, also of a .c file. > Provides: duktape-static = 1.2.2 Unusual. Those -static Provides are added to packages, which are build requirements themselves, *not* to packages that are linked statically. If a static version of duktape is linked with *and* bundled with the sources, just add Provides: bundled(duktape) = 1.2.2 to the package. > So that it can be tracked with a query of the packages. Such a query examines which -static packages appear in BuildRequires actually. Thanks Micahel. Gustavo, please follow Michael advice, and add Provides: bundled(duktape) = 1.2.2 duktape-static should be provided by a duktape package, that would probably consist of two files, named: %{_includedir}/duktape/duktape.{c,h} plus license and any doc files, and can be a very simple, next package for you :) Then, later you can make soletta "BuildRequires: duktape-static" glchaves's scratch build of soletta-0.0.1.beta10-1.fc22.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11598812 > In function '__fread_alias', > inlined from 'sol_memmap_write_raw_do.isra.2' at ./src/lib/io/sol-memmap-storage.c:169:15: > /usr/include/bits/stdio2.h:290:9: warning: call to '__fread_chk_warn' declared with attribute warning: fread called with bigger size * nmemb than length of destination buffer > return __fread_chk (__ptr, __bos0 (__ptr), __size, __n, __stream); > > It is caused by the fread in this code: > if (mask) { > uint64_t value = 0, old_value; > uint32_t i, j; > > for (i = 0, j = 0; i < entry->size; i++, j += 8) > value |= (uint64_t)((uint8_t *)buffer->data)[i] << j; > > ret = fread(&old_value, entry->size, 1, file); > > It looks suspicious. Shouldn't old_value be declared > as size_t, and instead of entry->size, write sizeof(old_value) > or sizeof(size_t) ? If the size is guaranteed to fit, it will > have issues in big endian architectures. I took a closer look and we're out of ideas on how to make gcc to shut up there. The assert() there was put in by the respective dev to make sure we got that space (we're under an if clause that guarantees that). I'll leave that as is for now. > ---%<--- > Another issue I noticed, is that %install appears to redo all > compilations, after regenerating header files. Some issue with > make rules? This one will be my next pursuit, ok? > Gustavo, please follow Michael advice, and add > Provides: bundled(duktape) = 1.2.2 > duktape-static should be provided by a duktape package, > that would probably consist of two files, named: > %{_includedir}/duktape/duktape.{c,h} > plus license and any doc files, and can be a very > simple, next package for you :) > Then, later you can make soletta "BuildRequires: duktape-static" Ok, I did like he advised, thanks. I can talk to duktape folks later, sure. All other review points were already adressed. New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v6/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v6/soletta-0.0.1.beta10-1.fc22.src.rpm koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=11598812 Hard to tell, as entry->size could be anything and isn't checked within that function. Can you enhance the build, so that the output is more verbose and shows the detailed invocations of gcc and ld? That would be the only way to tell how exactly the files are compiled/linked. Who knows under which circumstances results like https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=751812 turn up with GCC 5? > Version: %{soletta_major}.%{soletta_minor}.%{soletta_build}.%{soletta_tag} Hard to follow why it's done like that. As it ends up with soletta-0.0.1.beta10-1.fc22.src.rpm that doesn't follow the Package Versioning Guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Version_and_Release https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning Those would move the "beta10" out of %version and into a less-significant position in %release. That can be important for all cases where RPM version comparison is not compatible with upstream's release versioning scheme. Thanks, Michael. I agree, and if you check the history it was like the guidelines before Paulo asked me to change (Release: 0.1.%{soletta_tag}%{?dist}). Paulo, do you agree that we must revert to that? As for the entry->size thing, I'll double check again. I'll also add V=1 to our make invocation, so you guys see the exact tool invocation lines. Hi Gustavo and Michael. I just started a new fedora-review call, so will comment about other issues later. About versioning in release. The guidelines tell how to handle in a way that has less chances of having issues when upstream and rpm do not agree on versions, or what is newer. You can test it manually, for example with the command: $ rpmdev-vercmp 0.0.1.beta10-1 0.0.1.beta8-1 0.0.1.beta10-1 > 0.0.1.beta8-1 and what would happen if you called the final version 0.0.1: $ rpmdev-vercmp 0.0.1-1 0.0.1.beta10-1 0.0.1-1 < 0.0.1.beta10-1 Personally, I prefer to avoid messing the release tag, it is hard to maintain, and automated scripts cannot properly handle it. Michael, I was kind of trying to avoid too much complication on some packaging details, for the first package :) Gustavo, do you an ETA of when an official (non beta) package will be available? It would be desirable that the first official package in Fedora were not a beta one. But you can follow Michael advice on the release tag, then, after every package update, increase from 0.1.beta10 to 0.2.beta10 and so on. $ rpmdev-vercmp 0.0.1-beta10.1 0.0.1-beta8.222 0.0.1-beta10.1 > 0.0.1-beta8.222 $ rpmdev-vercmp 0.0.1-1 0.0.1-beta8.222 0.0.1-1 > 0.0.1-beta8.222 Personally I like the schema above, with just the beta tag + rel number to form the release. Can I go with that? The ETA for the 1st release is one month or less, we hope. The build issue was fixed locally, BTW. Hi Gustavo, While most times the warnings about spelling errors are pointless, there are these: %description flow-module-form This package contains the form flow module for %{name}. The modul eprovides nodes producing formatted, string output to feed LCD "modul" above is a typo, broke line at wrong position. ... %description pin-mux-module-galileo This package contains the galileo pin-mux module for %{name}. The module provides pin multiplexing rules/mapping for Galileo boards (revisions D ang G). Without this module, to use Galileo I/O pins with "ang" above should be "and"? -----8<----- The code in sol_memmap_write_raw_do still looks wrong. On little endian it will only work if reading 8 bytes, or, if initializing old_value to zero, and reading from 0 to 8 bytes. On big endian would work only if reading 8 bytes. If entry->size is larger than 8 bytes, it will trash the stack. The warning about result possibly not initialized in src/modules/flow/boolean/boolean.c appears sensible, as there is a code path where it can be used uninitialized, so, it should be safe to initialize it to zero. But you should know better :) -----8<----- About the version, It should be fine as you are upstream and also submitting the review, and increasing the beta number every review. Otherwise, it is common practice to add a "0.1" and increase it, to avoid the case of renaming the word "beta" in a way that would appear to be older. As long as you pay attention to rpmdev-vercmp it should be fine. You should notice any issues when making a test update package, *but*, sometimes it may be too late, as the package was already released, and then need to use the Epoch rpm tag to fix the mess :) -----8<----- I think we are almost done with the review. I would like to have an extra comment about my question about if you have an ETA for a stable release. For safety, in case you choose to call the first official release 0.0.1 it would be better to change back to Release: 0.1.%{soletta_tag}%{?dist} - but really, pre releases are avoided as official packages :) Not only for this reason, but due to not having a somewhat long lived, common release, so that people talk about the same thing :) It is not my duty to enforce the package versioning guidelines. I mentioned the guidelines, because %version 0.0.1.beta10 would be problematic, if there were a 0.0.1 final release. Same for a git snapshot made after 0.0.1 and before 0.0.2. Which %version to use then? The guidelines offer some flexibility for such scenarios. Discussing "the problem", mentioning the existing guidelines and tools like rpmdev-vercmp, may be sufficient. Still, occasionally packagers run into some of the pitfalls and then can't do much if they don't control upstream's versioning scheme. > $ rpmdev-vercmp 0.0.1-beta10.1 0.0.1-beta8.222 > 0.0.1-beta10.1 > 0.0.1-beta8.222 > $ rpmdev-vercmp 0.0.1-1 0.0.1-beta8.222 > 0.0.1-1 > 0.0.1-beta8.222 There are many schemes that seem to be compatible with RPM versioning comparison at first glance. For example, if comparing numbers with letters, numbers win always, so going from release beta8 to 1 seems to work. It can get awkward, if you publish builds for multiple distribution releases. Suddenly, the "beta8" at the most-significant left side of %release decides whether a package is newer than a build for another dist target. If you ever needed to rebuild a package for an older dist more often than for a newer dist, a different scheme is needed as not to compare build version with %{?dist}. It can also get troublesome, if you want to switch from a "beta" release to a git snapshot or vice versa. A real release number at the very left opens more opportunities to move pre-release and post-release version details to the less significant right side of %release. However, some of the problems are not encountered often, and an upstream versioning scheme that is fully compatible with RPM version comparison won't cause any problems. > and automated scripts cannot properly handle it. rpmdev-bumpspec as used during mass-rebuilds, handles Fedora's pre-release and post-release versioning schemes just fine. The only requirement is to avoid macro-madness in the Release tag. There are only a very few packages, which would win every spec file obfuscation contest and confuse rpmdev-bumpspec. > I think we are almost done with the review. I would like to have
> an extra comment about my question about if you have an ETA for
> a stable release. For safety, in case you choose to call the
> first official release 0.0.1 it would be better to change back
> to Release: 0.1.%{soletta_tag}%{?dist} - but really, pre releases
> are avoided as official packages :) Not only for this reason, but
> due to not having a somewhat long lived, common release, so that
> people talk about the same thing :)
Hi, Paulo. For me it's also more reasonable to push the RPM release button
starting from our actual 1st (not pre) release, so let's work with that.
For the next iteration here, that might happen before our upstream release,
disregard the beta tags, which will vanish in sequence. I'll work on
the remaining issues and help with dev here and soon I'll come back
with updates. Thanks a lot, Paulo and Michael!
Hi Gustavo, since this review has been a bit silent, I did run a full review again, so, removing some noise of "fedora-review -b 1264546" on latest rawhide, updated today: ---8<--- [?]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "*No copyright* GPL (v2 or later) (with incorrect FSF address)", "GPL (v3 or later)", "Unknown or generated", "PSF (v2)", "MIT/X11 (BSD like)", "BSD (3 clause)". 423 files have unknown license. Detailed output of licensecheck in /home/pcpa/1264546-soletta/licensecheck.txt I noticed there are some files with Python license: soletta-1_beta10/src/modules/flow/converter/LICENSE.PSFL soletta-1_beta10/src/modules/flow/string/LICENSE.PSFL Well, actually, only the python license, the sources there appear to all use BSD/MIT like license. There is also soletta-1_beta10/tools/kconfig/zconf.tab.c_shipped with a GPLv3+ license. But I wonder if there any issues regenerating it, to have the parsed file distributed with sources. ---8<--- [?]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. I understand this may easily get outdated on updates, but still is a good procedure to document it, mostly to show you are aware of all the source contents. Usually, it is comments about sources, e.g.: # BSD License: # some/dir/* # some/other/dir/* (not built/used) # MIT License: # some/dir/* # GPLv2+ # some/dir/* License: GPLv2+ and BSD and MIT And/or also on files section: %files # GPLv2+ /file/paths ---8<--- [?]: Package contains no bundled libraries without FPC exception. For the moment I believe this is not an issue. Just do not install headers or .so in -devel packages. ---8<--- [?]: Package functions as described. I will trust you, and %check for that :) ---8<--- [?]: Latest version is packaged. I believe were waiting for the first release. ---8<--- [?]: Package should compile and build into binary rpms on all supported architectures. I understand you were working on ensuring the package builds in arm. ---8<--- [?]: Packages should try to preserve timestamps of original installed files. I kept this as unchecked as there are issues in the make process, where it rebuilds everything on make install. Thanks a lot, Paulo! I'll be updating the package soon (now for real), with all comments taken into account. We've been rushing with our v1 backlog and I waited a little for the features to get in to package again. This week I'll come back to Fedora packaging. I'll still use tags in the package naming and, again, ignore that for now (that will vanish for the final release). glchaves's scratch build of soletta-0.0.1-beta13.1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12055290 glchaves's scratch build of soletta-0.0.1-beta13.1.fc23.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12055502 New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v9/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v9/soletta-0.0.1-beta13.1.fc23.src.rpm koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=12055502 Paulo, the warning on the coap code is already fixed here and will vanish in the next iteration. The warning on that memmap code is still there, but as we guarantee it won't happen, we have an assert to check that the size of the destination buffer is enough. All other issues were fixed, I hope. We'll take some time to reach 1.0 (think beginning of 2016). The version+release tag is in a way that at least conforms to rpmdev-vercmp. So it's up to you guys if we can proceed with a beta package or wait to the release. Thanks a lot for the support :) Ping. Have you had the time to take a look, Paulo? I can bump the spec to newer code before that if you prefer too, no big deal. We're closer to the 1st release now, and it would be nice to have all packaging issues gone ASAP :) Thanks a lot. (In reply to Gustavo Lima Chaves from comment #55) Hi Gustavo, > Ping. Have you had the time to take a look, Paulo? I can bump the spec to > newer code before that if you prefer too, no big deal. We're closer to the > 1st release now, and it would be nice to have all packaging issues gone ASAP > :) :) Nice to know it is getting close to a final release. I will make an extra informal review of the 12/04 package now, and should give feedback shortly. > Thanks a lot. Hi Gustavo. The package looks pretty good now. There is still the issue of it building twice, but since the package builds fast, and you know already know about it, it is not a blocker. I am only in doubt about usage of PSF for license, so I asked @packaging about it, as it might need to use PSFL, or follow the pattern of other packages, that use simply "Python" when referring to PSF(L). I will let you know about any news I have about it. Thanks, Paulo! I'll wait for the verdict on the Python Licensing citation, then. Sad that the build issue is still there, we'll put more effort to fix that for the next iteration (soon I'll have one posted). Cheers. The next iteration is coming soon. We're really rushing towards 1.0 here, and since we had agreed on only packaging with a release (out of beta state), I'm focusing more on getting to it than updating beta specs, OK? But we're almost there now, just as a heads up. https://github.com/solettaproject/soletta-packaging/pull/26 awaiting review, now v1.0 ack-ed on our side, with rpmlint happy for all assets. Now going for Fedora's infra back again (koji build). New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v11/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v11/soletta-1-1.fc24.src.rpm koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=15389537 Hi, Paulo. Finally, we have come to this -- v1 both on our side and on the RPM spec. Sorry for taking longer than needed, but a lot of development took place in order for this to happen. We're now a stable API, at least, and hope to benefit many Fedora users with the ease of packaging. There are close to 0 rpmlint complaints, I hope we're good now. The libmicrohttpd issue has been solved by pushing its usage to future releases, when the lib version in the system has been bumped. Let me know if there are any issues left. 1. Please add an owner to the directory /usr/share/soletta/flow/aliases 2. I suggest creating a ChangeLog file, and installing it under /usr/share/doc/soletta-devel. The rpm spec %changelog is not development changelog :) See https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs specifically: """ The intent is to give the user a hint as to what changed in a package update without overwhelming them with the technical details. """ It is not a blocking issue, just that it is not the right place for development changelog. Most times a changelog file can be created on the fly with "git shortlog", but you may prefer to have a more well formatted one. Please consider the comments above for a minor update before the final build. I consider the package approved. Gustavo, Welcome as a Fedora packager! Please see remaining information at https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner I will be your sponsor. If you have any issues, please send me an email, or for more urgent issues you can also ping me in freenode, nickname pcpa. I will do some extra duties as sponsor, like watching you bugzilla interactions, but that is just for the bureaucracy part :) Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/soletta Thanks a lot, Paulo! I'll do the alias file reference, sorry for that (rpmlint failed me on that, I think :/). As for Changelogs, can I simply edit the ones I got, removing non-RPM packaging relevant stuff (with no new changelog entries on the spec)? Hi Gustavo, About the changelog, feel free to to how you think is better. For now there isn't much logs in the changelog, but if you kept that way, soon you could have a really large changelog :) New spec URL: https://raw.githubusercontent.com/solettaproject/soletta-packaging/v12/rpm/soletta.spec New SRPM URL: https://github.com/solettaproject/soletta-packaging/releases/download/v12/soletta-1-1.fc24.src.rpm koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=15507592 Please let me know if we still got issues or if further instructions on how to proceed are yet unknown to me. Thanks a lot. soletta-1-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-a45444cb7d soletta-1-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-8f80075e7f soletta-1-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-8f80075e7f soletta-1-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-a45444cb7d soletta-1-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. Well, I'm still waiting for the Fedora 24 testing request. Also, on rawhide, we're getting hit with errors on builds for the new aarch64 target -- a floating point issue detected by our tests. We'll have to attack that on a subsequent release. This closed review should not keep blocking FE-NEEDSPONSOR. soletta-1-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. |