https://bugzilla.redhat.com/show_bug.cgi?id=1670656
--- Comment #10 from Mark Goodwin <[email protected]> --- (In reply to Xavier Bachelot from comment #8 and comment #9) ... > > Unfortunately, as with most web apps, all required nodejs deps are not > > available in Fedora .. > > And if no one packages them, it will not change. Chicken and egg :-) true, however the are literally zillions of nodejs packages - the task of packaging them as RPMs and maintaini9ng them is impossible. A better approach may be to set up a moderated fedora npm registry and allow builds to access that. But for now it seems, webpack is the best we have. > > > Meanwhile, you should add versioned Provides: bundled(nodejs-blahblah) = > > > x.y.z for each of the bundled module. done. > > > Same for go modules, indeed. > > > https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling > > > > The grafana golang code seems to be self contained, or only uses go libs > > that are already included by the BR for golang. > > > Ok. It might still be a good idea to explicitly list all Requires: > that are needed at runtime. For example, perl guidelines mandates that. After install, 'ldd /usr/sbin/grafana-server' shows there are no go libraries required at run-time so likely no additional Requires are needed. > > > - You don't need to create %{buildroot}/%{_unitdir}, it's already there > > > because of the BR: systemd. > > > > it seems to be needed, at least for local builds (where BUILDROOT may not > > have been populated with all BR directories outside of /). Official Fedora > > builds probably don't needed it. > > > This is not the "supported" way to build. Mock is the way to go, even for > local builds, this is much safer. > Anyway, your call, this is not a blocker. ok yep - I should be building with mock, rather than just running rpmbuild directly. I am also building in copr (mgoodwin/grafana), which doesn't need %{buildroot}/%{_unitdir} created either. I added a comment. > > > - The rundir needs to be handled by tmpfiles.d > > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Tmpfiles.d/ > > > > this one is confusing - what's currently there seems to be working. > > > Yes, it is working, but is not the way mandated in the guidelines. OK done, now installing %{_tmpfiles}/grafana.conf for /run/grafana/grafana.pid So it will now be created again after a reboot. > > ...snip... > > > > - Still about doc, you should build the content of docs/ and ship it, > > > possibly in a separate sub-package if it is too big' > > > > yes I have considered a subpackage, but grafana.org themselves do not > > ship the docs. Instead they are available online and there are links in > > the grafana UI to browse the documentation. I also have added man pages for > > grafana-server(1) and grafana-cli(1) which mention the URLs to the > > online documentation. > > > Still makes sense to ship them, imho. I can easily imagine a setup where the > grafana instance doesn't have access to the outside world. > Maybe the URls also need to be patched to point at the local install ? I'm starting to become out of time - perhaps we could add a -doc subpackage in a later update. > > I've tried my first build of the rpm then ran rpmlint. > - There are a lot of files which are shipped with exec bit set, triggering > lots > of spurious script-without-shebang and wrong-script-end-of-line-encoding > errors. I found the cause of that: installing with mode 0755 the entire public directory (so even the files were mode 755, clearly wrong). No fixed, and rpmlint is much happier - just a false positive on some svg files and a few other things. > - I believe most if not all of /usr/share/grafana/scripts/build/ is not > needed. > Fixing these 2 items should shorten rpmlint output quite a lot. scripts are now omitted - they're not needed for the shipping package afaict. > > Other rpmlint stuff : > > grafana.x86_64: E: non-readable /etc/grafana/grafana.ini 640 > grafana.x86_64: E: non-readable /etc/grafana/ldap.toml 640 expected > > In the spec, there is : > install -p -m 644 conf/distro-defaults.ini > %{buildroot}/%{_sysconfdir}/%{name}/grafana.ini > install -p -m 644 conf/ldap.toml %{buildroot}/%{_sysconfdir}/%{name}/ldap.toml > but later > %config(noreplace) %attr(0640, root, %{GRAFANA_GROUP}) > %{_sysconfdir}/%{name}/grafana.ini > %config(noreplace) %attr(0640, root, %{GRAFANA_GROUP}) > %{_sysconfdir}/%{name}/ldap.toml > > So, 640 or 644 ? Choose one and make the spec consistent. > 640 should be choosen if there is any security sensitive information in the > file, 644 if not. > Also, if the perms on a file or dir are already correct you don't need to > specify it again in %attr and can use '-' instead. > eg.: %attr(-,root,grafana) should be 640 root/grafana since the config and sysconf have a couple of default passwords and need to be readable by the grafana-server process, but only writeable by root. > Now, parsing the specfile again. > > %define GRAFANA_USER %{name} > %define GRAFANA_GROUP %{name} > %define GRAFANA_HOME %{_datadir}/%{name} > > --> use %global fixed > # license and other package docs, but not APP docs (they're online) > install -p -m 644 LICENSE.md %{buildroot}/%{_defaultlicensedir}/%name > install -p -m 644 README.md ROADMAP.md CHANGELOG.md PLUGIN_DEV.md > %{buildroot}/%{_docdir}/%{name} > install -p -m 644 CODE_OF_CONDUCT.md CONTRIBUTING.md NOTICE.md > %{buildroot}/%{_docdir}/%{name} > then in %files : > # other docs and license > %doc %{_datadir}/doc/%{name} > %license %{_defaultlicensedir}/%name/LICENSE.md > > --> Just use the following in %files, the %doc and %license macro take care > of installing the files in the proper location : > %license LICENSE.md > %doc CHANGELOG.md CODE_OF_CONDUCT.md CONTRIBUTING.md NOTICE.md > %doc PLUGIN_DEV.md README.md ROADMAP.md UPGRADING_DEPENDENCIES.md The %doc macro isn't finding those files correctly when they're named individually - it cd's to the wrong directory. So I've left this as-is for now, as follows %doc %{_datadir}/doc/%{name} %license %{_defaultlicensedir}/%name/LICENSE.md > > %pre/%post/%postun don't need the '||:' safeguard, this is already taken care > of by the %systemd_* macros ok, removed > Also, usually, the scriptlet sections are before the %files section. > ok, moved scriptlets to before %files so now %files is just before changelog. > > Also, about the run dir, I just checked again and /run is on tmpfs, hence the > need for using tmpfiles.d. > fixed now, as mentioned above. > # other shared files, public html, webpack and scripts > cp -rpa conf public scripts %{buildroot}/%{_datadir}/%{name} > --> -a implies recursive and preserve so 'cp -a' should be enough. fixed. > > Source1: grafana_webpack-%{?version}.tar.gz > --> spurious ? in macro, version is always set. fixed. > > > %global __brp_ldconfig /bin/true # no libs > --> what is the purpose of this line ? to avoid the following warning message when ldconfig is run by rpmbuild : /sbin/ldconfig: Warning: ignoring configuration file that cannot be opened: /home/mgoodwin/rpmbuild/BUILDROOT/grafana-5.4.3-6.fc28.x86_64/etc/ld.so.conf: No such file or directory I've removed it now since it seems like a bit of a hack, and not related to grafana per-se. > Sorry for the quite messy comment, it started clean, but then I started > to raw dump any potential issue. I hope it makes sense and is useful anyway. all makes sense, thanks for the on-going reviewing. Here's the rpmlint now: rpmlint /home/mgoodwin/rpmbuild/RPMS/x86_64/grafana-5.4.3-6.fc28.x86_64.rpm grafana.x86_64: W: non-standard-gid /etc/grafana/grafana.ini grafana grafana.x86_64: E: non-readable /etc/grafana/grafana.ini 640 grafana.x86_64: W: non-standard-gid /etc/grafana/ldap.toml grafana grafana.x86_64: E: non-readable /etc/grafana/ldap.toml 640 grafana.x86_64: E: non-readable /etc/sysconfig/grafana-server 640 grafana.x86_64: E: non-readable /usr/share/grafana/conf/defaults.ini 640 grafana.x86_64: E: script-without-shebang /usr/share/grafana/public/app/plugins/datasource/elasticsearch/img/elasticsearch.svg grafana.x86_64: E: script-without-shebang /usr/share/grafana/public/fonts/grafana-icons.svg grafana.x86_64: W: hidden-file-or-dir /usr/share/grafana/public/sass/.sass-lint.yml grafana.x86_64: W: hidden-file-or-dir /usr/share/grafana/public/test/.jshintrc grafana.x86_64: W: non-standard-uid /var/lib/grafana/data grafana grafana.x86_64: W: non-standard-gid /var/lib/grafana/data grafana grafana.x86_64: W: non-standard-uid /var/log/grafana grafana grafana.x86_64: W: non-standard-gid /var/log/grafana grafana grafana.x86_64: W: log-files-without-logrotate ['/var/log/grafana'] 1 packages and 0 specfiles checked; 6 errors, 9 warnings. New reference URLs after all of the above changes: Spec URL: https://raw.githubusercontent.com/goodwinos/grafana/native-rpm-spec/packaging/rpm/spec/grafana.spec SRPM URL: http://people.redhat.com/~mgoodwin/grafana/grafana-5.4.3-6.fc28.src.rpm COPR URL: https://copr.fedorainfracloud.org/coprs/mgoodwin/grafana/ Note: after installing, prior to starting the service, you'll need to: sudo chgrp grafana /usr/share/grafana/conf/defaults.ini (that file is supposed to be readable by group grafana - I'll fix it in the morning) Cheers -- 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 -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected]
