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

Bug#1055041: marked as done (RFS: a2d/2.0.3-1 -- APRS to DAPNET portal)

Your message dated Sat, 4 Nov 2023 18:04:44 +0100
with message-id <ZUZ5rEpjW/+EAf+M@frost.de>
and subject line Re: Bug#1055041: RFS: a2d/2.0.3-1 -- APRS to DAPNET portal
has caused the Debian Bug report #1055041,
regarding RFS: a2d/2.0.3-1 -- APRS to DAPNET portal
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1055041: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1055041
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: sponsorship-requests
Severity: normal
X-Debbugs-Cc: kd8mbd@gmail.com

Dear mentors,

I am looking for a sponsor for my package "a2d":

 * Package name     : a2d
   Version          : 2.0.3-1
   Upstream contact : Yogeswaran Umasankar <kd8mbd@gmail.com>
 * URL              : https://github.com/NGC2023/a2d
 * License          : MIT, CC-BY-3.0
 * Vcs              : https://salsa.debian.org/yogu/a2d
   Section          : hamradio

The source builds the following binary packages:

  a2d - APRS to DAPNET portal

To access further information about this package, please visit the following URL:

  https://mentors.debian.net/package/a2d/

Alternatively, you can download the package with 'dget' using this command:

  dget -x https://mentors.debian.net/debian/pool/main/a/a2d/a2d_2.0.3-1.dsc

Changes since the last upload:

 a2d (2.0.3-1) unstable; urgency=medium
 .
   * New upstream version.
   * Fixed failed VCS repository. (Closes: #1055040)

Regards,
-- 
  Yogeswaran Umasankar

--- End Message ---
--- Begin Message --- 
Am Wed, Nov 01, 2023 at 09:32:49AM -0400 schrieb Yogeswaran Umasankar:
> Hi Tobi,
> On Wed, Nov 01, 2023 at 10:23:54AM +0100, Tobias Frost wrote:
> > Control: tags -1 moreinfo
> > 
> > Hi Yogeswaran,
> > 
> > I've took a look at your package:
> 
> Thank you for reviewing the package, I appreciate your feedback and
> comments.
> 
> > - You do not replace the debian changelog, you append the new
> >  information. (hint: use "debchange" or "dch" for changelog manipulation)
> 
> I am using visual studio code to do all the modifications before using
> gbp for packaging. That maybe the reason its replacing the changelog.
> Unless its an issue, I would like to use visual studio code for now.

You can use any tooling you want, as long as the result is correct ;-)
(gbp can create changelog entries out of git commits, maybe that works
with your workflow)

> > - there are undocumented changes, for example it seems that dependencies
> >  has been changeed. that (and why it is) should be noted in
> >  d/changelog.
> 
> I have fixed all the undocumented changes, and noted all of them in
> debian changelog.
> 
> > (You are also upstream)
> 
> Thank you for the feedback on upstream, it helped to improve the
> upstream.
> 
> > - It seems you have renamed a2dapp to a2d. This change is not good,
> >  as it is too short / generic, and renaming is a breaking change
> >  for your users. I'd appreaciate if you could stay with the old name.
> 
> I am not renaming the package from a2dapp to a2d. The original package
> name was a2d from the begining. And a2dapp was not defined properly in
> the previous version in setup.py (not according to Debian python). So in
> upstream I have fixed it by moving the flask files to original a2d, and
> also updated to toml instead of setup.py.

Ok, thanks for explaining

> > - Do you really	 need an hard dependency on nginx?
> >  (I've checked myself: I think you don't. you'll use nginx as a proxy,
> >  but people might want to setup e.g another http server for that or
> >  even directly connect to the 9333 port (can they?); I think nginx
> >  should be Recommends: not Depends: -- see policy about the definition
> >  of Depends.)
> 
> Yes we need hard dependency on nginx. a2d is a flask app and it is
> designed in a way that flask handles reverse proxy setup all the way to
> generating signed CA SLL for the user. Without nginx the whole flask
> will crash. It is by design to make it easy for ham radio users with
> limited experience in linux systems. I have provided detailed
> instructions for the users in the flask app on how changing ports and
> working on SSL will impact their system.

Let me rephrase my question: Does it need a web server or does it need
the web server to be nginx? *Can* it work with apache or lighgttpd, for example?
(I do not mean "out of the box", but more *can* it work, when configured appropiatly)

The configuration you ship is of course a nice config to get started, but will
complicates if users have a not that simple setups, e.g when there is already a
webserver on the machine your package is installed. It would be nice to cater
something like that too.

For Letsencrypt, there is software in Debian for handling certificates,
it would be preferable to use this one.

The message I try to convey is that in Debian we try very hard not to limit
use cases, we try to avoid that if a package is installed other usecases won't
be possible anymore, and we put effort into that to integrate software wel into
the distribution. I'm not knowing a2d well enough to say whats possible, but
e.g the hardcoding of nginx is not within that spirit…

> > - You README.md suggests to do rm -rf <various directories>
> >  That is bad advice! Do not blindly say that they should nuke the nginx
> >  configuration; (If, then the user should run apt purge nginx, but this
> >  is also still dangerous a people might loose data); deluser is also
> >  a bad advice, (system) users should not be deleted after they have
> >  been created.
> 
> I have added clear warning to the user in the README.md file about
> removing nginx. I have removed the part about deluser.
 
> > - postinst is wrong. Especially you do not remove those files on
> >  "remove" -- only purge should delete user configuration, for example.
> > 
> > Thinking about it I am not sure if you are handling conffiles correctly,
> > as your postrm will nuke also files not handled by your packge, by
> > blindly removing them. This is an RC bug. Probably your application
> > will create files there? Maybe than /etc/ is the wrong location, maybe
> > app-created files should be reside in /var/lib/a2d? I'm not sure here
> > if/how this is handled correctly, maybe someone else will chime in.
> > I'm just sure that "rm -r /etc/a2d" could make some admins unhappy.
> 
> I believe you are talking about postrm, postinst dont remove anything.
> (1) As you suggested, I have fixed the postrm so only during purge it
> deletes user config and user's a2d database.
> (2) I placed the user generated config files in /etc/, here the user use
> flask to create their config files. So I believe it should be in /etc/
> and not in /var/lib/a2d.

Yes, it was postrm; silly me mistyped when reportinb the bug ;-)
Thanks for fixing the isuse (#1055150).

> > (I'm running out of time here, the review might not be complete)

One more thing: a2d seems to be ran as root. This is suboptimal for
security purposes. Maybe you have ideas to refactor a2d to not needing
root privileges ;-)

> Thank you again for your time!
> Yogeswaran Umasankar.

As the package is an improvement about the current one in unstable,
I will upload the package after sending this mail.

Many thanks for your contribution to Debian!

-- 
cheers,
tobi

Attachment: signature.asc
Description: PGP signature


--- End Message ---
Reply to: