Bug#832941: RFS: 4pane (debian: message 7 of 20)
On Sat, Oct 08, 2016 at 09:37:48AM -0700, Sean Whitton wrote: > On Fri, Sep 30, 2016 at 03:40:33PM +0100, David Hart wrote: > > >2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386? > > I didn't think it would. However after discussion on debian-mentors I've now > > shown it works on armel, and it builds on kfreebsd-amd64 but immediately > > hangs > > in a forkpty call; I'll try to debug that when I have time. I expect some or > > all of the other archs will also build. If/when 4pane attracts a sponsor > > I'll > > try them. I haven't looked at all at 4pane, but it might be a case of grantpt()/forkpty() failing to work if you have a handler for SIGCHLD: https://github.com/kilobyte/kbtin/commit/f915c121a19c00bd52dd7e159778806e8dba0979 (the commit message says freebsd kernel 9.x, but that's as in "as opposed to 8.x". And it's glibc only, real FreeBSD works ok.). (My fix has an obvious race condition, you need another wait after restoring the signal.) > Debian packages need to work on all our release architectures unless > there is some fundamental fact about the package that prevents it > (e.g. a tool for analysing machine code on a particular arch). Sadly that's not true, packages are allowed to skip any architectures for any reason or no reason at all. The only case it's an issue is when the architecture worked before (to be exact: it has a version in testing) and you haven't yet filed a RM. Being allowed and it being the right thing to do are different things, though, and I'd urge you to port to all architectures if possible. > Note that kfreebsd-amd64 is not a release architecture anymore. On one hand, this means less reasons to make it work, but on the other, the porters are swamped with extra effort and could use any help you can do. Making your package work there is always nice (and, unlike exotic archs, you can trivially set up kfreebsd in a virtual machine or on a spare partition at native speed). Meow! -- A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg raspberries, 0.4kg sugar; put into a big jar for 1 month. Filter out and throw away the fruits (can dump them into a cake, etc), let the drink age at least 3-6 months.
Bug#832941: RFS: 4pane (debian: message 7 of 20)
Dear David, A few comments on your reply. On Fri, Sep 30, 2016 at 03:40:33PM +0100, David Hart wrote: > >2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386? > I didn't think it would. However after discussion on debian-mentors I've now > shown it works on armel, and it builds on kfreebsd-amd64 but immediately hangs > in a forkpty call; I'll try to debug that when I have time. I expect some or > all of the other archs will also build. If/when 4pane attracts a sponsor I'll > try them. Debian packages need to work on all our release architectures unless there is some fundamental fact about the package that prevents it (e.g. a tool for analysing machine code on a particular arch). Note that kfreebsd-amd64 is not a release architecture anymore. > >4 I think that it would be clearer to express the wxWindows license as a > >dual-license: "GPL-2+ or wxWindows", with separate license paragraphs for > >each > >of those. > I'm not sure I understand. wxWidgets isn't dual-licensed, it uses just the > wxWindows license which, though similar to GPL-2, is different. I might be missing something, but the text of the license says: - you can use the library under the GPL-2 or later - you can use the library under wxWindows License 3.1 or later - if you add other GPL code to the library you can no longer distribute under the wxWindows License - if you make modifications you can continue the dual-licensing or not The 3rd and 4th points follow from the 1st and 2nd, so all the license is actually saying is the 1st and 2nd, i.e., a dual license under GPL-2+ and wxWindows-3.1+. > > Of your suggestions, 2, 3, 5, and 7-9 are now implemented. Of the > others that need a comment: > > >1 I'd be very grateful if you'd put this source package in a git repository. > I presume you mean just the contents of debian/. I've done this: > https://github.com/dghart/4pane-debian-dir I'd recommend including the upstream code in the git repository, too. But what you've done is fine -- thanks! > >6. Can Vcs-Git: use a secure protocol? https:// rather than git://. > I don't think so, not without the user logging in to sourceforge. Fair enough. > >11. I'm not familiar with wxWidgets practices, but is it unavoidable to > >include sections of code from the wxWidgets sources... > It's not at all normal, but I have to do it in two places: > First, originally the main display widget was effectively unsubclassable and > even now it would be very hard. I hope to replace the control with one new in > wx3.0 when I no longer support wx2.8. Second, wxWidgets' inotify wrapper is > new > in wx3.0; I've copied some of the code for use in earlier versions. Sounds reasonable, though, I'm not a C++ programmer. Hopefully someone else on d-mentors will comment. > >12. Please consider using dh_autoreconf to ensure that the package's > >build system can be reproduced from the source code provided > If I understand dh_autoreconf correctly, it can't: a fresh autoconf call will > fail as the package doesn't include various non-standard .m4 macros and such. > That could be fixed by a largish patch, but I'll leave it to a sponsor to > decide. In that case, this is now a "must-fix". The build system is part of the source package, so the source code for that build system must be included, to satisfy DFSG. You need to include those macros (or switch to use standard ones). It is not required to run dh_autoreconf: it is sufficient to include everything that would be needed to reconfigure. However, it is strongly recommended that you actually reconf, because it proves that all the required source code is actually present. Further, running dh_autoreconf can uncover bugs in the build system. -- Sean Whitton signature.asc Description: PGP signature
Bug#832941: RFS: 4pane (debian: message 7 of 20)
Dear Sean, Many thanks for taking the time to write such a detailed review. Of the must-fixes: >1. The changelog should contain exactly one entry, closing the ITP. Ah, that not only makes sense but is easier. Done. >2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386? I didn't think it would. However after discussion on debian-mentors I've now shown it works on armel, and it builds on kfreebsd-amd64 but immediately hangs in a forkpty call; I'll try to debug that when I have time. I expect some or all of the other archs will also build. If/when 4pane attracts a sponsor I'll try them. >3 The Vcs-* fields should point to a repo/branch containing the source >package, not the upstream master branch. That also makes sense. Done. >4 I think that it would be clearer to express the wxWindows license as a >dual-license: "GPL-2+ or wxWindows", with separate license paragraphs for each >of those. I'm not sure I understand. wxWidgets isn't dual-licensed, it uses just the wxWindows license which, though similar to GPL-2, is different. Of your suggestions, 2, 3, 5, and 7-9 are now implemented. Of the others that need a comment: >1 I'd be very grateful if you'd put this source package in a git repository. I presume you mean just the contents of debian/. I've done this: https://github.com/dghart/4pane-debian-dir >4.Why is there an additional copy of the manpage in the debian/ subdir? That was a quick-fix for a 4[Pp]ane issue. I've now removed it and instead use a link. >6. Can Vcs-Git: use a secure protocol? https:// rather than git://. I don't think so, not without the user logging in to sourceforge. >10. You're inconsistent about 4pane versus 4Pane... The original name was 4Pane, and outside debian it still is. I don't mind changing things while packaging but it risks breakages, so I'll ask my future sponsor how best to do this. >11. I'm not familiar with wxWidgets practices, but is it unavoidable to >include sections of code from the wxWidgets sources... It's not at all normal, but I have to do it in two places: First, originally the main display widget was effectively unsubclassable and even now it would be very hard. I hope to replace the control with one new in wx3.0 when I no longer support wx2.8. Second, wxWidgets' inotify wrapper is new in wx3.0; I've copied some of the code for use in earlier versions. >12. Please consider using dh_autoreconf to ensure that the package's build >system can be reproduced from the source code provided If I understand dh_autoreconf correctly, it can't: a fresh autoconf call will fail as the package doesn't include various non-standard .m4 macros and such. That could be fixed by a largish patch, but I'll leave it to a sponsor to decide. Again, thank you for your time and effort. Regards, David Hart >Dear David, > >I've split it into two sections: things that I would consider must-fixes >before an upload to Debian, and suggested improvements. The latter >aren't strictly necessary, but they would help demonstrate to a >potential sponsor that you are committed to maintaining this package in >Debian. > >Must-fixes/clarifications: > >1. The changelog should contain exactly one entry, closing the ITP. The >current changelog suggests that 4pane has already been uploaded to >Debian. > >2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386? >(an eclectic list!) > >3. The Vcs-* fields should point to a repo/branch containing the source >package, not the upstream master branch. > >4. I think that it would be clearer to express the wxWindows license as >a dual-license: "GPL-2+ or wxWindows", with separate license paragraphs >for each of those. > >Suggestions: > >1. I'd be very grateful if you'd put this source package in a git >repository. That way, I can use `git diff` to review changes that >you've made in response to my comments. > >2. At least PACKAGERS, README are missing a trailing newline. > >3. The "but may be used by others" line in the manpage doesn't make much >sense. Normally this is used to indicate that a manpage written by a >Debian contributor is used in a Debian derivative, but of course any >copy of a manpage written by *upstream* gets used by others. > >4. Why is there an additional copy of the manpage in the debian/ subdir? > >5. "As well as standard file manager things" I would suggest >s/things/functionality/. Also, the scare quotes around 'terminal >emulator', 'grep' etc. aren't needed and could be confusing. > >6. Can Vcs-Git: use a secure protocol? https:// rather than git://. > >7. The debian/* paragraph in d/copyright is redundant. > >8. The Debian menu system is deprecated. You seem to be installing a >FreeDesktop.org desktop file into /usr/share/4Pane/rc. Please install >that as a desktop file that will be picked up by desktop environments -- >see Debian Policy 9.6. > >9. I think the html docs should go in /usr/share/doc/4pane/html. Then >someone just looking for the changelog, README etc. need not hunt
