-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add debian package #43
Comments
Help is wanted here. I have a .deb built, but I would like someone else to take responsibility for actually uploading. Also, there's a missing dependency (python-reedsolo) |
Hi! I found your project in Debian's "WNPP" archives: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1021089 There I asked if you needed help with this, before finding this issue, so I guess I'm here to propose our help!
So I guess I'll look at packaging python-reedsolo next! :) |
and that's https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=985095 and i'm building a package now. |
On 2023-02-27 11:21 am, anarcat wrote:
Hi!
I found your project in Debian's "WNPP" archives:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1021089
There I asked if you needed help with this, before finding this issue, so I
guess I'm here to propose our help!
Normally, when you file an "ITP" (Intent To Package) as you did, you intend
to do the packaging yourself and it's unlikely someone will pick up that
work unless you explicitly ask for help or it "times out" (and turns into an
"RFP", a Request For Package).
So I guess I'll look at packaging python-reedsolo next! :)
I tried to get a sponsor on #debian-python with no luck. Thanks for following
up!
Both qr-backup and python-reedsolo are at
https://germinate.za3k.com/pub/debian/, in addition to the source for
qr-backup packages you already found.
|
On 2023-02-27 12:45:49, Zachary Vance wrote:
I tried to get a sponsor on #debian-python with no luck. Thanks for following
up!
Both qr-backup and python-reedsolo are at
https://germinate.za3k.com/pub/debian/, in addition to the source for
qr-backup packages you already found.
oh. bad luck there: i have repackaged reedsolo already! i have just
uploaded it to the archive and it should land in NEW shortly.
please review the package here to see if it's up to what you would have
expected:
https://salsa.debian.org/python-team/packages/reedsolo/-/tree/debian/master/
|
also, about the qr-backup debian package itself... i see you have the source package stuff in |
Yes, but probably in a couple days. If you shoot me a PR I'll approve it
tonight.
…On 2023-02-27 12:52 pm, anarcat wrote:
also, about the qr-backup debian package itself... i see you have the source
package stuff in package/debian... if you move that up one level, you'll
have that package pretty much in the right shape already... it's something
I'd have to do to fix the package before upload, any chance you could tweak
that already? :)
--
Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
You are receiving this because you authored the thread.Message ID:
***@***.***>
Links:
------
[1] #43 (comment)
[2]
https://github.com/notifications/unsubscribe-auth/AAFRLUUPQ2KTDXL5TZQJNR3WZUHZFANCNFSM6AAAAAAQUH6YMM
|
This is a more standard way of doing things.
done, #46 |
This was a dead link when I tried it: https://salsa.debian.org/python-team/packages/reedsolo/-/tree/debian/master/ |
how about https://salsa.debian.org/python-team/packages/python-reedsolo? someone had already done the packaging and had a better version, which we decided was better than mine. it's waiting in NEW now. |
i looked at the source code around here a little bit, and i think it generally looks sane, but there's lots of room for improvement. one thing that could be improved is the manpage stuff. in i'm not sure what to do about the embedded font. why is it there? font-dejavu is already in debian and basically every linux distribution out there, it seems like dead weight... i'm not sure it's completely against policy but typically we try to avoid vendored code copies, i'm just not sure it applies to fonts. we could repackage the source to remove it, but that's more work for debian and divergence from upstream which we rather avoid. there's also two LICENSE files and the debian/copyright which seem redundant... the qr-backup code itself is a little messy and hard to read, a little all over the place. the QR class looks like a missed opportunity to regroup a bunch of functions as class methods instead. i would use logging.debug almost everywhere you use logging.info and add a --debug flag. i would suggest using argparse. i would add type hints as well, as right now it's really hard to tell where data is coming from and whether scanning an unknown set of codes could cause security issues. in the restore test, you pass the pasphrase to gpg directly, on the commandline, which is a security liability, as some other user on the same system could sniff the passphrase from the process list. but really, none of this should be a blocker for debian packaging. the copyright file and embedded font stuff might get the package rejected, but maybe that's a bridge we can cross then... i hope you don't mind the review: i typically do an audit of the source code before uploading to debian for security reasons and i hope it's useful for you. :) |
On 2023-03-03 9:11 am, anarcat wrote:
i looked at the source code around here a little bit, and i think it generally looks sane, but there's lots of room for improvement.
one thing that could be improved is the manpage stuff. in docs/ you have qr-backup.1.man and MAN.txt, which is a little confusing. is the former generated by the latter? if so, that should probably be done at build time and qr-backup.1.man (ideally) removed from git. now i see there's a hidden thing to generate those, shouldn't that be in the makefile instead? and while i'm talking about the makefile, it seems to me this would be better served by a setup.py or setup.cfg or pyproject.toml or whatever it is the latest thing python packaging does. :)
The makefile was added only for debian packaging, basically.
i'm not sure what to do about the embedded font. why is it there?
font-dejavu is already in debian and basically every linux distribution out there, it seems like dead weight... i'm not sure it's completely against policy but typically we try to avoid vendored code copies, i'm just not sure it applies to fonts.
The font is not included in the built debian package, just the source. qr-backup is designed as much as possible to ship so it runs on a computer from 20 years ago with no special packages. (It falls short in many regards)
there's also two LICENSE files _and_ the debian/copyright which seem redundant...
debian/copyright being redundant seems like a problem debian has created :)
IIRC I packaged the debian stuff following the recommendations of the debian wiki. If you want to update that, it would be a public good, probably. You could mention "how to go from packaging it locally to having in be in debian" especially.
the qr-backup code itself is a little messy and hard to read, a little all over the place. the QR class looks like a missed opportunity to regroup a bunch of functions as class methods instead. i would use logging.debug almost everywhere you use logging.info and add a --debug flag. i would suggest using argparse. i would add type hints as well, as right now it's really hard to tell where data is coming from and whether scanning an unknown set of codes could cause security issues.
The qr-backup code IMO is hurt by being in one file, but we want that for good reasons.
Type hints are a good idea, I've been avoiding them for silly reasons.
Argparse would not make this code better. Renaming --verbose as --debug might be reasonable. IIRC The reason I don't use logging.debug is that one of the modules I'm using prints a BUNCH of garbage to debug. Which is a dumb reason, do you know how to not print that from imports? I guess I could use a non-default logger or something?
but really, none of this should be a blocker for debian packaging. the
copyright file and embedded font stuff might get the package rejected, but
maybe that's a bridge we can cross then...
i hope you don't mind the review: i typically do an audit of the source code before uploading to debian for security reasons and i hope it's useful for you. :)
This review doesn't tell me much I don't know, but I certainly don't mind it. Your other issues you opened have been very useful.
Thanks for your work on making qr-backup better and for helping package it. Sorry I'm a bit non-programmer-mode atm, I really do intend to get to the issues you raise.
Edit: Fixed horrible linebreaks
|
On 2023-03-03 09:37:54, Zachary Vance wrote:
On 2023-03-03 9:11 am, anarcat wrote:
[...]
The makefile was added only for debian packaging, basically.
You really don't need a Makefile to make a Debian package. Debian is
happy to package software build with setuptools, poetry, etc...
> i'm not sure what to do about the embedded font. why is it there?
> font-dejavu is already in debian and basically every linux distribution out
> there, it seems like dead weight... i'm not sure it's completely against
> policy but typically we try to avoid vendored code copies, i'm just not sure
> it applies to fonts.
The font is not included in the debian package, just the source. qr-backup is
designed as much as possible to ship so it runs on a computer from 20 years
ago with no special packages. (It falls short in many regards)
Yeah so the trick here is there are two types of Debian packge. What you
refer to here is the `.deb`, the "binary" package. But Debian also ships
"source" packages which will include the font unless the tar file is
"repackaged", as I mentioned.
> there's also two LICENSE files _and_ the debian/copyright which seem
> redundant...
debian/copyright being redundant seems like a problem debian has created :)
Right. So it really depends on how you want to do this. If you want to
take care of the debian package, this may be a "native" package in which
case it makes sense to use only one copyright file. But typically, we
package stuff separately from upstream and manage a separate
debian/copyright that has a specific, "machine readable" format that
upstream don't necessarily want to follow.
In this case, you *can* remove this duplication, as an upstream, but you
don't *have* to. What I was pointing out was mostly redundancy between
LICENSE and LICENSE.md.
IIRC I packaged the debian stuff following the recommendations of the debian
wiki. If you want to update that, it would be a public good, probably. You
could mention "how to go from packaging it locally to having in be in debian"
especially.
It's a very long story, but to keep it simple: Debian is a very old
project and there are many ways of doing things. If you point me to a
specific wiki page that you think led you astray, I'm happy to review
and discuss it. :)
> the qr-backup code itself is a little messy and hard to read, a little all
> over the place. the QR class looks like a missed opportunity to regroup a
> bunch of functions as class methods instead. i would use logging.debug
> almost everywhere you use logging.info and add a --debug flag. i would
> suggest using argparse. i would add type hints as well, as right now it's
> really hard to tell where data is coming from and whether scanning an
> unknown set of codes could cause security issues.
The qr-backup code IMO is hurt by being in one file, but we want that for
good reasons.
Yeah, I don't think that's the biggest problem here.
Type hints are a good idea, I've been avoiding them for silly
reasons.
Same here. I avoided them for a long time, but I started using mypy
recently, and it's really a game changer.
Argparse would not make this code better.
Maybe not, but it would make easier for external contributors to grok
your code. :)
Renaming --verbose as --debug might be reasonable. IIRC The reason I
don't use logging.debug is that one of the modules I'm using prints a
BUNCH of garbage to debug. Which is a dumb reason, do you know how to
not print that from imports? I guess I could use a non-default logger
or something?
I've faced that problem before and yes, there are ways. It really
depends on how the library code is architectured. Ideally, libraries
have their own logger that you can tweak levels for. Which library is
it?
> but really, none of this should be a blocker for debian packaging. the
> copyright file and embedded font stuff might get the package rejected, but
> maybe that's a bridge we can cross then...
> i hope you don't mind the review: i typically do an audit of the source code
> before uploading to debian for security reasons and i hope it's useful for
> you. :)
Your suggestions have been useful in general. The review doesn't tell me much
I don't know, but I certainly don't mind it.
Thanks for your work on making qr-backup better and for helping package it.
Sorry I'm a bit non-programmer-mode atm, I really do intend to get to the
issues you raise.
Hey it's alright. :) Better to put stuff out there and improve it
incrementally than wait for perfection and never do anything!
…--
If we do not do the impossible, we shall be faced with the unthinkable.
- Murray Bookchin
|
python-reedsolo works great. Thanks for packaging it, and sorry for the long delay in getting back to this. |
Great, should we get started on that? I will take a look at the suggestions and get started on those, but like you say I don't think they should be a blocker. Let me know if the embedded font becomes a problem.
https://wiki.debian.org/Packaging/Intro?action=show&redirect=IntroDebianPackaging Why don't you just take a look, and see if there is anything you thing needs updated? I think I can be out of the loop on this one, since I've forgotten most of the context in the last 6 months.
Opposite problem here. I worked at Dropbox, and we were Guido's beta testing guinea pigs for mypy. I kept worrying that stuff I knew would be out of date from the "final" version that shipped. |
that looks moderately reasonable.
i am a bit too busy right now, not sure i will have much cycles for this unfortunately. |
Darn, very reasonable six months later though. Thanks for packaging python-reedsolo at least |
No description provided.
The text was updated successfully, but these errors were encountered: