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

Bug#861581: ITP: rainloop -- Simple, modern & fast web-based email client



Hi Daniel Ring,

Noticed your ITP announcement on debian-devel and it seemed your package
could be a useful addition. I might sponsor you if needed (but like I
tell everyone I offer to sponsor please go through the regular RFS
procedure and CC me just so others can also review your changes and
sponsor your uploads when I'm too busy).
I noticed Gunnar has already offered to review and sponsor but I figured
the more the merrier, right? ;)
Thus adding some quick comments from my perspective below in case they
are useful.

On Sun, May 07, 2017 at 11:01:33AM +0000, Daniel Ring wrote:
> May 2, 2017 9:58 AM, "Gunnar Wolf" <gwolf@debian.org> wrote:
> > Hi Daniel,
> > 
> > I'm interested in looking at your package. When it's ready and when
> > you need a sponsor, mail me!
> 
> Hi Gunnar,
> 
> I've finished putting together a preliminary version of the package, but I
> have a few concerns about it.
> 
> The largest one is that the build system is NodeJS-based, and requires a
> version of npm newer than the one currently in Debian. Bugs #857986 and #794890
> have some details about npm's issues. Installing nodejs from its official
> repository works, as does building on Ubuntu.

Hopefully also npm from experimental works?

As you might be aware npm was removed from Stretch as requested by
its maintainer, since not enough people where helping out getting it
in good enough shape to be in a stable release. Unless the situation
improves it's likely npm won't be in next stable (Buster) either (and
I haven't seen anyone really offering to help yet)...

In my view, maintaining a package also means you need to look at the
health of your dependencies and get them in shape where needed.
Are you interested in getting involved with packaging of npm itself
or how do you view the current outlook of not being included in
next stable release?

If you're not targeting getting your package shipped in a stable release
then it can still be useful, but you might also find peoples interest
in reviewing and sponsoring to be less enthusiastic.


> 
> Secondly, the build system has the usual issue with NodeJS packaging; it
> downloads dependencies at runtime. Most of the packages don't exist in Debian
> or are out of date, and I found several existing packages doing this while
> looking for a better solution, so I'm not sure how much of an issue this is.

Me neighter, but in general I think you should assume atleast some people
will have problem with anything else than you packaging those dependencies
as separate (reusable) packages. See the gitlab packaging efforts as an
example. If you bundle unpackaged stuff that might be ok up until too
many packages ship a bundeled version and security team eventually vetos
not including your package. Thus if you bundle it's likely either way
good to see this as flying under the radar and always being on the lookout
for alternative ways to handle this so you're prepared for the day when
you might have to handle it (or preferably even have it handled before that).

> This only occurs at build-time, and nodejs isn't required to use the software.

I guess you're already aware though that (atleast on official Debian buildds)
there's no internet connectivity available at package build time...

> 
> Finally, the upstream source contains several embedded libraries. I was able to
> swap a few of them for existing packages in Debian, but there are a few PHP
> libraries that don't have existing packages. The JavaScript libraries are
> amalgamated into a single file at build-time, and separating them out would be
> a non-trivial amount of work for decreased performance. Again, I found several
> existing packages doing this, so I'm not sure how much of a problem it is.

Me neighter, but preferably the amalgation process should be a step in
the package building. One reason is it's easier to fix any future found
issues by patching the source and rebuilding rather than having to patch
something generated.

> Upstream provided sources for most, I added the few missing to satisfy lintian.
> 
> I've uploaded the package to mentors: https://mentors.debian.net/package/rainloop

(Thanks for the link. Full .dsc url usually preferred.)

> Please review it when you have a chance, and let me know if there's anything I
> need to fix!
> 

I've quickly looked at the packaging and in general it looks well prepared.
I made some notes about things that popped up on my mind attached below.
One more general question I have is about security though. See for example
roundcube which has had quite a few CVEs found and fixed during the years.
Has rainloop taken any particular stance on development practises for
security? How new is the project or how widely has it been deployed yet
that might give it some kind of practical security track record?
Anyway, my notes...


debian/control:
- "nodejs (>=6) | npm (>=3)" b-dep looks weird to me as these two are
  not functionally equivalents of each other.
- please use (the more secure and preferred) https url for Vcs-Git
- Mix of php(7) and php5 dependencies? Only php5 compatible? Will we
  ship php5 or will rloop soon be php7 compatible?
  For example the php-sabre-vobject dependency in turn seems to
  depend on php(7.x). Maybe I'm just uninformed and some php5-* packages
  are both 5 and 7 compatible?!

debian/copyright:
- You probably need to include the full AGPL-3 license text since it's not
  available in "common-licenses".
- Please consider, since it might make your life easier, to license
  debian/* under the same license as upstream (consider any contributed
  patches that might end up under debian/patches/* which you'll likely
  want to upstream to lessen your own maintenance burden).

debian/*postinst:
- Have you considered excluding /var/lib/rainloop from dh_fixperms
  instead of using maintainer script?
  The www-data user is a statically allocated user which should have the
  same uid on all Debian systems. (See 'base-passwd' package.)


Hope this helps.

Regards,
Andreas Henriksson


Reply to: