Hi Maytham, On Fri, Nov 10, 2023 at 01:38:05PM +0800, Maytham Alsudany wrote: > On Thu, 2023-11-09 at 22:53 +0530, Nilesh Patra wrote: > > > 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.
No worries, thank you for pushing this. I am convinced that this serves no real purpose so I removed in from d/rules. > > 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? Yes, if you could open an issue, it'd be nice! > 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. It is hard for me to believe that you are contributing to debian package for the first time, as all your contributions seem to be of high quality which is not something I see in newcomers, so kudos for that! > 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? As such no, all packages in the debian/ namespace as such belong to a particular language and we will keep involving maintainer teams for all such packages. IMHO there's nothing these two teams would do together for such a package, and I'm (very) active in both the teams so I say this with some confidence :) > > > 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? Absolutely! > 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 As such I don't think the debian namespace has got anything to do with this. You could just fork it to your own :) > 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) I think this is a little hurried and upstream may not even want something like that. I suppose it'd be more sensible to first *ask* the upstream as to what they think about the concerns and we can go about sending patches once we reach some concensus. > > > > > [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 Best, Nilesh
signature.asc
Description: PGP signature
