Hi Nilesh, Here's what I thought of the discussion. On Thu, 2023-11-09 at 22:53 +0530, Nilesh Patra wrote: > > It would probably be beneficial if we could setup the gbp workflow by hand, > > ensuring the upstream branch is up to date > > Not sure what you mean here. `gbp import-orig` takes care of it. My mistake, I thought that the gbp workflow can only be setup with a new git repo and a dsc. Having a look at the gbp docs has cleared this up for me. > > and setting up a patch-queue/debian/(unstable|experimental) branch. > > No, please. These are never really meant to be pushed. Besides people use different > way to apply patches - I use quilt and not `gbp pq`. Thanks for clarifying that matter, it was a bit unclear to me. (I had pushed the patch-queue branch to [2] and [3] thinking that was the way to go.) > > Regarding commit 7020dd5d: > > Are the ldflags necessary, since the binary builds fine without them set? > > Especially the kitty.VCSRevision option, which is only really used in --version, > > as shown in this upstream commit[9]. > > I do not find any commit at [9] but rather the lintian output. I simply > decided to emulate the way in which upstream buildsystem would behave to > avoid surprises so I'd be OK with keeping it that way. LMK if you > disagree. Sorry, wrong link. I meant [10]. But I don't think this applies anymore, as the tools/cli/infrastructure.go file has been removed. I cloned the upstream repo, built it using setup.py, and passed the build option `--vcs-rev=VCS_REV_GOES_HERE`. After running `strings ./kitty/launchers/kitty | grep -i VCS_REV_GOES_HERE`, and the same with the kitten binary, no mention of the VCS revision was found, except for the following in "kitten": build -ldflags="-X kitty.VCSRevision=VCS_REV_GOES_HERE" build -ldflags="-X kitty.VCSRevision=VCS_REV_GOES_HERE" So I don't think it matters whether we pass the VCSRevision in ldlags or not, nor what value we pass, since it's not being used. I agree with passing the other ldflags though, in order to emulate setup.py and the upstream buildsystem. Forgive me for overemphasizing this, but I wanted to get to the bottom of this. > > > > Should we add a simple recursive find and replace to debian/rules [to change > > the shebangs from python to python3]? WDYT? > > The problem with such a call is that the package can't build again > after the first build due to non-patch changes in the files. So if we > are doing a recursive replace, it should be done in the > "override_dh_python3" step on the files *inside* debian dir. > > OTOH, it may be possible to do so "dh_python3 > --shebang=/usr/bin/python3" magic to get this fixed. > > To be clear, recursive find and replace is OK for me as long as it is > done in a later step inside the files in debian/ dir. Feel free to go > ahead with a fix. Done. I've added a recursive find and replace to override_dh_auto_install in debian/rules, which goes through the debian/tmp/usr/lib/kitty directory that is copied beforehand. > That said I do see a directive to "/usr/bin/env python3" in specific > files for instance in "kittens/tui/handler.py" "kitty/fonts/render.py" > but not in others. IMHO we should ask upstream to maintain uniformity by > fixing this. I agree. Should we open an issue and/or PR upstream? > > > * Since we both are interested in kitty's packaging, I think we have two > > > options: > > > - Either you or me would be in the "Maintainer" field and the other one would > > > be in "Uploader" field. > > > - Add kitty@packages.debian.org as the maintainer and add both of us > > > as uploaders (that means we subscribe to the package email > > > ofcourse). > > > Which one do you think we should do? > > > > I don't really have a preference, so I'm happy with whatever you would like to > > do regarding that. > > So I suppose the question now changes to: "Would you like to be > responsible to maintain this package and commit to its maintenance as a > primary PoC"? > > > Keep in mind that the contact in the Maintainer field will > > pretty much be the "face" of the package. > > I've maintained a fair bit of python and go packages for a few years > even before I was a Debian Developer so I'd be kind of OK with being the > maintainer - ofcourse only if you're fine with it. I'm fine with that. You've got more experience, whereas this is my first time contributing to a Debian package, so I reckon you should be the "Maintainer" of the package, if you're fine with it. Just a thought: should we get the Python and Golang teams involved, since both programming langs are being used and both dh buildsystems are being used? > > > * I suppose the maintenance of this package will keep getting messy due > > > to upstream mixing up two language build systems in a fairly non-standard way. > > > I suppose it makes sense to ask upstream if they'd consider to switch > > > to something that eases maintenance burden on us (Debian). WDYT? > > > > I think that's a good idea, but if we do ask upstream, we should propose a > > solution to them to make it as easy as possible for them to implement, and > > perhaps some work on our side. > > > > Perhaps splitting up the kitty and kittens components into separate > > folders/repos? Or make it so that these components can build independently of > > each other (setup.py builds kitty only, `go build` builds kittens only)? > > Yes, I agree. While I was doing the work of making the go part work, I > felt that it'd indeed be more sensible if "kitten" was a separate repo > altogether. Should we bring this up on upstream's issue tracker? Another option would be to split the repos ourselves on GitHub (under the Debian org), which requires more work on our part but is more likely to get the upstream author to accept our proposal: 1. Fork github.com/kovidgoyal/kitty to github.com/debian/kitty 2. Remove all "kitten" components from the fork 3. Create github.com/debian/kitten and move all the Golang "kitten" components to it 4. Open a PR to merge debian/kitty to kovidgoyal/kitty 5. In the PR, offer to transfer ownership of debian/kitten (or let the upstream author just copy/fork it themselves) > > > > [1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1037440#37 > > > > [2]: https://salsa.debian.org/Maytha8/kitty-dh-golang > > > > [3]: https://salsa.debian.org/Maytha8/kitty > > > > [4]: https://src.fedoraproject.org/rpms/kitty/blob/rawhide/f/kitty.spec#_268 > > > [5]: https://salsa.debian.org/debian/kitty/-/tree/debian/experimental?ref_type=heads > > [6]: > > https://salsa.debian.org/debian/kitty/-/blob/15a00c9b73f1008520c49f8c795f3bada3eed171/setup.py#L972 > > [7]: > > https://salsa.debian.org/debian/kitty/-/blob/15a00c9b73f1008520c49f8c795f3bada3eed171/debian/patches/0003-build-golang-components-with-dh-golang.patch > > [8]: https://github.com/kovidgoyal/kitty/issues/5473 > > [9]: > > https://debian.pages.debian.net/-/kitty/-/jobs/4902978/artifacts/debian/output/lintian.html [10]: https://github.com/kovidgoyal/kitty/commit/d39036d Let me know your thoughts Nilesh! Kind regards, Maytham
Attachment:
signature.asc
Description: This is a digitally signed message part