[Bug 1670656] Review Request: grafana - an open source, feature rich metrics dashboard and graph editor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1670656



--- Comment #8 from Xavier Bachelot <xavier@xxxxxxxxxxxx> ---
Hi Mark,

Thanks for the update.
I've stripped all the points that don't need to be discussed anymore to
concentrate on the ones that are still open.

(In reply to Mark Goodwin from comment #7)
> (In reply to Xavier Bachelot from comment #6)

...snip...

> > Also, I'm not sure to which extend such bundling is allowed or not, but I'll
> > leave it alone for now and defer to people who know better.
> > I know from first hand experience that unbundling may consume a lot of time,
> > and keeping it that way is likely the best short term solution.
> 
> Yes, it is still a bit debatable, though I'm confident this is the right
> approach.
> Using a pre-built webpack is how cockpit is (intending to) handle the nodejs
> dependencies problem, so this is how grafana can too. There are other
> packages
> heading this in the same way too, e.g. vector and others.
> 
> 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 :-)

> and so tools such as npm, yarn and grunt need
> to be network-enabled so they can fetch packages from an npm registry.
> As you know, Fedora build policy does not allow this. Yarn can be told
> to use a pre-downloaded cache of node modules, but a webpack is better.
> Using a webpack is a reasonable compromise - it allows dependent node
> packages
> to be downloaded apriori, security checked by the maintainer, then bundled
> into the webpack and shipped with the app. The webpack is obfuscated and
> stamped
> with the build-id and then included as a Source tarball in the package.
> In a sense a webpack can be considered a version stamped private library of
> "compiled javascript" for use by the application. We thus get secure offline
> reproducible builds that yeild QE'able packages - this is clearly consistent
> with
> tried-and-tested, sane Fedora packaging principles.
>
I agree to the compromise, but I wouldn't be against a stronger wording in
these guidelines.

> > Meanwhile, you should add versioned Provides: bundled(nodejs-blahblah) =
> > x.y.z for each of the bundled module. 
> 
> Since the bundled javascript is in a private webpack, is this still
> necessary?
> Only grafana will be using this webpack and so the grafana package does
> not "provide" any nodejs modules for any other package. The webpack is
> basically a shahash'ed private static library.
>
Yes, it is, for security audit purpose.
Say, a security issue is spotted in nodejs-foo. A bug will be filed against
nodejs-foo package and the security issue will be patched in an updated rpm,
which will benefit all packages using it as a dep.
For bundled deps, the Provides: bundled(nodejs-foo) allows to query the
repository for such things and also notify maintainer of packages that bundle
it.
If there's no Provides: bundled(nodejs-foo), there's no way to accomplish this
task.
Again from first hand experience, relying on upstream to monitor and fix issue
in the libs they are bundling is not a safe bet.

> > 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. 

> > - 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.

> > 
> > - 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.

...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 ?

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
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/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux