On Thu, 2023-11-09 at 00:32 +0530, Nilesh Patra wrote: > > On the contrary, avoiding the use of dh-golang as done in this repo[3] causes > > all the tests to pass without problem, and I'm unsure to why that is. > > This was due to some caveats with the build system and also how > dh_golang works. We added in a patch that'd skip running gen-go-code.py > and this was being used at more than one place. Doesn't the update_go_generated_files function in setup.py[6] run gen-go- code.py, specifically `kitty +launch gen-go-code.py`? Patch#0003[7] adds the `return` statement right afterwards. > I've fixed up the build and the tests and the package seems to > build/work. I suppose we should be pushing it to debian experimental for > now since this introduces some completely new things. Let me know if you > disagree. Not at all! I completely agree with uploading it to experimental (once we get all the issues reported by lintian resolved), since it a large version bump and we've changed a lot of things to get the Golang parts to build. > I've pushed all my changes to the debian/experimental branch on the > existing salsa repo[5] and also added your access to it so you could > push directly. Thanks, especially for adding and pushing my commits! > > We may have to take the approach Fedora has taken, where they've skipped any > > continuously failing tests[4]. > > For now I disabled two tests in the go code that tries to fiddle with > proc/devfs and can potentially fail in a chroot. Python tests probably > also try to do some non-standard stuff and we could disable it later if > it goes flaky on the buildd machines. If we have time, it's probably good to get to the bottom of these flaky tests and patch them instead of skipping them outright. This may involve filing bug reports upstream, and creating PRs when necessary upstream in order to improve their test suites for both Python and Golang. These issues regarding the tests have been brought up before with upstream. For instance, James opened an issue[8] on upstream's GitHub regarding the test_bash_integration test failing on unstable. > That said, I want to discuss/ask a few things: > * Can you take a look at my commits and let me know if you have any > comments? Regarding commit 38ea7407: This is a nitpick (and I think this is a mistake): should the patch file end with .patch instead of .py? It would probably be beneficial if we could setup the gbp workflow by hand, ensuring the upstream branch is up to date and setting up a patch- queue/debian/(unstable|experimental) branch. Regarding commit d847dbfe: The d/ch entry needs some cleaning up, with the merging and removal of some points. (I'm assuming you used `gbp dch`.) For example, > * patch: make setup.py compatible with GOPATH mode is probably not something the end user needs to know about. Let me know if I should leave it or clean it up. 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]. > * Can you please clean up some of the lintian stuff? And then we could > upload the new release. The latest lintian report from the (newly discovered) GitLab Salsa pipeline can be found here[9]. I can go through and fix most of the issues lintian reports, but I would like discuss with you the unusual-interpreter warning in the kitty binary package. All the kitty/**/*.py files are using a `#!/usr/bin/env python` shebang, whereas lintian wants `#!/usr/bin/env python3`. Should we add a simple recursive find and replace to debian/rules? WDYT? > * 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. Keep in mind that the contact in the Maintainer field will pretty much be the "face" of the package. > * 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)? > > [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 Kind regards, Maytham
Attachment:
signature.asc
Description: This is a digitally signed message part