Spec URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user.spec SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user-1.4.3-1.fc18.src.rpm Description: Provides a remote-user auth service based plugin for OpenShift Fedora Account System Username: tdawson
rpmlint output: rpmlint rubygem-openshift-origin-auth-remote-user.spec rubygem-openshift-origin-auth-remote-user-1.4.3-1.fc18.src.rpm rubygem-openshift-origin-auth-remote-user-1.4.3-1.fc18.noarch.rpm rubygem-openshift-origin-auth-remote-user-doc-1.4.3-1.fc18.noarch.rpm rubygem-openshift-origin-auth-remote-user-doc.noarch: W: non-conffile-in-etc /etc/openshift/plugins.d/openshift-origin-auth-remote-user.conf.example 3 packages and 1 specfiles checked; 0 errors, 1 warnings. - Upstream doesn't want to mark that file as a conffile, because it is an example file.
Why is there a favicon in source2 ? ( since that's used only for test ? )
That was to get rid of the rpmlint empty file errors.
Spec URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user.spec SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user-1.5.6-2.fc18.src.rpm - Updated to the latest stable release - Updated spec file to work with new F19+ requirements.
Why is there a %post to change the configuration of a file that is not in the package ? %post if [ $1 -ne 1 ] # this is an update; fix the previously configured realm. then sed -i -e 's/AuthName.*/AuthName "OpenShift Broker API"/' /var/www/openshift/broker/httpd/conf.d/openshift-origin-auth-remote-user.conf fi Package seems to be not installable on rawhide at the moment due to ruby 2.0 ( but I need to investigate )
Just a few points, mostly without explanation, since they are the same as in bug 894524 :) * %{_docdir}? what is that? shouldn't it be %{gem_docdir}? * exclude %{gem_cache} * %gem_install macro * vendor? * %{gem_libdir} instead of %{gem_instdir}/lib * %{gem_docdir} and %doc marking.
Spec URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user.spec SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user-1.5.6-3.fc18.src.rpm - %post script - removed. This was for an old conf format used back when this was first started. - %{_docdir} docs moved to %{gem_docdir}. - exclude %{gem_cache} - Done - %gem_install macro - Used - vendor - You must have gotten this mixed up with the other openshift rubygem. I couldn't find any directory and/or file with that name in it. - %{gem_libdir} instead of %{gem_instdir}/lib - Done - %{gem_docdir} and %doc marking - again, I think you have this mixed up with the other rubygem spec file. * I will work on upstream to get these fixed upstream. * Builds and installs on rawhide (F20)
(In reply to comment #7) > - %{_docdir} docs moved to %{gem_docdir}. Ups, now I read the .spec file more carefully and get the original idea with the %{_docdir} and I must say that it was correct idea. Little explanation would be enough, since I was confused :) However the approach was a bit clumsy. It should be enough to do: %install install -m 644 %{SOURCE1} . %files %doc %{name}-doc.README I apologize for confusion :/ > - vendor - You must have gotten this mixed up with the other openshift > rubygem. I couldn't find any directory and/or file with that name in it. > - %{gem_docdir} and %doc marking - again, I think you have this mixed up > with the other rubygem spec file. Sorry, it might be. It was just quick fly through :) I found another nit: you should use %{_sysconfdir} instead of /etc
And I forgot to ask you about test suite ...
Spec URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user.spec SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user-1.5.6-4.fc18.src.rpm Installed the doc.README as you suggested. I looked into the test suite. It requires part of the openshift enviroement needs to be setup for it to work. In particular it needs a mongodb setup. Doesn't look like the test suite is do-able in an rpm enviroment.
Technically, it could be done ( after all, if we can make a X server work ... ), but maybe that's too complex. No way to run only part of the test ?
OK, at this point I'm going to have to say "really?" I have to wait months at a time in between comments. The ruby build requirements have changed twice while waiting, so I have to repackage according to the new ruby build requirements. (Though I have to say the new macro's make things much cleaner.) The package has updated twice during that time. Testing is an optional thing for ruby packages (at least last time I checked). The purpose of this package/plugin is to do authentication for openshift users. Those users are supposed to be in an openshift mongodb database. Can I write up something that installs a mongodb database, adds some test openshift users in it, then test against that. I suppose I could. If I had some assurance that that is where it stops, maybe I'd try. But it's really starting to feel more like your looking for reasons not to pass my package than to help me get the package right.
My apologies. I shouldn't have said that. I'm in a bit of a time crunch for this package and it's starting to show. As I said above, the purpose of this package/plugin is to do authentication for openshift users. Those users are supposed to be in an openshift mongodb database. Could I write some type of setup for doing this, possibly. I'm not a database guy, but I could work on it. But it would take alot more time that I feel I have. I would really like this package because I use it in my tests for the OpenShift Origin feature testing in F19. Time is currently short on that, so I'm feeling a bit under pressure for this package. If we could get this package reviewed without the tests I would appreciate it. Thank You
For sure, we can get without the tests, but given the changing nature of ruby, I strongly recommend to have them enabled later and try to spend sometime on having a way to make them work. I will see if I can provide a patch for that. Anyway, I am not up to date regarding the changes to the ruby packaging policy, but there is a item in fedora review for : "Gem package should exclude cached Gem." So can you exclude the cache from the %files list ? https://fedoraproject.org/wiki/Packaging_talk:Ruby#Cached_.gem_file Another issue is the requires. The gemspec talk of The spec say : Requires: %{?scl:%scl_prefix}rubygems Requires: %{?scl:%scl_prefix}rubygem(json) Requires: %{?scl:%scl_prefix}rubygem(rails) Requires: rubygem(openshift-origin-common) Requires: rubygem(openshift-origin-controller) Requires: openshift-origin-broker Gemspec only speak of : s.add_dependency('openshift-origin-controller') s.add_dependency('json') After looking at the code, there is no need for the json module ( except for test ). It seems I forgot to make PR for that after seeing 6 months ago : https://github.com/mscherer/origin-server/commit/982375a196d11f887097c751189e032ccdfa9700 Anyway, this package should be good, provided thse 2 minors changes are fixed so approved. Package Review ============== Key: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed [!]: Gem package should exclude cached Gem. [!]: No need to requires ruby json ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package requires other packages for directories it uses. Note: No known owner of /var/www/openshift [x]: Package must own all directories that it creates. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf %{buildroot} present but not required [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Fully versioned dependency in subpackages, if present. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rubygem- openshift-origin-auth-remote-user-doc [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local Ruby: [x]: Platform dependent files must all go under %{gem_extdir}, platform independent under %{gem_dir}. [x]: Gem package must not define a non-gem subpackage [x]: Gem package is named rubygem-%{gem_name} [x]: Package contains BuildRequires: rubygems-devel. [x]: Gem package must define %{gem_name} macro. [x]: Pure Ruby package must be built as noarch [x]: Package contains Requires: ruby(abi). ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [-]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. Ruby: [!]: Gem package should exclude cached Gem. [!]: Test suite of the library should be run. [x]: Specfile should use macros from rubygem-devel package. Note: The specfile doesn't use these macros: %{gem_libdir}, %exclude %{gem_cache} ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: rubygem-openshift-origin-auth-remote-user-1.5.6-2.fc18.noarch.rpm rubygem-openshift-origin-auth-remote-user-doc-1.5.6-2.fc18.noarch.rpm rubygem-openshift-origin-auth-remote-user-doc.noarch: W: non-conffile-in-etc /etc/openshift/plugins.d/openshift-origin-auth-remote-user.conf.example 2 packages and 0 specfiles checked; 0 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint rubygem-openshift-origin-auth-remote-user rubygem-openshift-origin-auth-remote-user-doc rubygem-openshift-origin-auth-remote-user-doc.noarch: W: non-conffile-in-etc /etc/openshift/plugins.d/openshift-origin-auth-remote-user.conf.example 2 packages and 0 specfiles checked; 0 errors, 1 warnings. # echo 'rpmlint-done:' Requires -------- rubygem-openshift-origin-auth-remote-user (rpmlib, GLIBC filtered): /bin/sh openshift-origin-broker ruby(abi) rubygem(json) rubygem(openshift-origin-common) rubygem(openshift-origin-controller) rubygem(rails) rubygems rubygem-openshift-origin-auth-remote-user-doc (rpmlib, GLIBC filtered): /usr/bin/env Provides -------- rubygem-openshift-origin-auth-remote-user: rubygem(openshift-origin-auth-remote-user) rubygem-openshift-origin-auth-remote-user rubygem-openshift-origin-auth-remote-user-doc: rubygem-openshift-origin-auth-remote-user-doc MD5-sum check ------------- http://mirror.openshift.com/pub/openshift-origin/source/rubygem-openshift-origin-auth-remote-user/openshift-origin-auth-remote-user-1.5.6.gem : CHECKSUM(SHA256) this package : 83f8017b9dcd79811680c7b3d3dbc45fed32e1646cbf93a198ff1d384d7883ea CHECKSUM(SHA256) upstream package : 83f8017b9dcd79811680c7b3d3dbc45fed32e1646cbf93a198ff1d384d7883ea Generated by fedora-review 0.4.0 (cf29f98) last change: 2013-02-08 Buildroot used: fedora-18-x86_64 Command line :./try-fedora-review -b 909471
Spec URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user.spec SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/rubygem-openshift-origin-auth-remote-user-1.5.6-5.fc18.src.rpm - Remove rubygems(json) dependancy - exclude gem_cache
Mh, I tought I had set fedora-review to + already, but it was not.
New Package SCM Request ======================= Package Name: rubygem-openshift-origin-auth-remote-user Short Description: OpenShift plugin for remote-user authentication Owners: tdawson maxamillion Branches: f17 f18 f19 InitialCC:
Git done (by process-git-requests).
rubygem-openshift-origin-auth-remote-user-1.5.6-5.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/rubygem-openshift-origin-auth-remote-user-1.5.6-5.fc18
(In reply to comment #12) > OK, at this point I'm going to have to say "really?" Troy, although it may be annoying, I strongly encourage everybody to include test suite, even some basic one. It helps a lot during times like this, i.e. new Ruby is released and there is done automatic rebuild, or even during mass rebuild of Fedora, since your package may become failing anytime, due to changed dependencies, etc. So it is safe net for every Fedora user. I as a maintainer am glad, if the test suite fails during such occasion and I am happy to fix any issue it uncovers. Naturally, I am happier if I see that although there were changes, the test suite still passes. Also, please note, I am not sure how much are you involved in upstream development of this package, but this is upstream question IMO, so there should be no hard feeling. It should be no blocker, but I have to ask. > I have to wait months at a time in between comments. Sorry for that, but you can always ping me.
rubygem-openshift-origin-auth-remote-user-1.5.6-5.fc18 has been pushed to the Fedora 18 testing repository.
rubygem-openshift-origin-auth-remote-user-1.5.6-5.fc18 has been pushed to the Fedora 18 stable repository.