Hello Samuel, Many thanks for a dedicated reply. I walked through your comments and tried to address all of them. For the latest changes, have a look at the following repository: https://salsa.debian.org/dfukui/libewf/-/commits/debian/master
To make review easier, here are the changes from the previous draft. https://salsa.debian.org/dfukui/libewf/-/compare/debian%2F20140813-1-old...debian%2Fmaster?from_project_id=69076 Following are my replies to your comments. > 4) debian-changelog-line-too-long: > 5) no-nmu-in-changelog and source-nmu-has-incorrect-version-number: > 6) spelling-error-in-changelog Debian Debian (duplicate word): All of them are addressed. > 7) bad-exception-format-in-dep5-copyright expat with public-domain: I tried to replace the license with a new one "Expat and public-domain", but it looks like lintian does not recognise it as a single term for some reason. For more details, take a look at: https://salsa.debian.org/dfukui/libewf/-/jobs/3079374#L30 https://salsa.debian.org/dfukui/libewf/-/jobs/3079374#L31 https://salsa.debian.org/dfukui/libewf/-/jobs/3079374#L40 I suspect this is just a false positive. That said, if this is really a problem and needs a fix, I will try another expression for that license. > 8) invalid-short-name-in-dep5-copyright: > 9) license-file-listed-in-debian-copyright COPYING: > 10) Files: debian/*: > 11) Files: manuals/ewfinfo.1 ...: > 12) unused-license-paragraph-in-dep5-copyright gpl3+ [debian/copyright:133]: > 13) Copyright: Copyright (C): All of these issues are addressed. > Great, the debian/ tag is not generally needed by the way, as the > sponsor is the one who should push that ideally, but I don't mind if > you push it. I know gbp will work smoother if you create a debian/ tag > (it lets you do gbp push without extra parameters if there's a tag). Thanks for pointing this out. OK, I understand that debian/ tag is not necessary in asking for a sponsor. > 12) older-debian-watch-file-standard 3: > 13) out-of-date-standards-version 4.5.0: > 14) silent-on-rules-requiring-root: > 15) debian-rules-uses-as-needed-linker-flag: All of the above are addressed. As for 14), two different types of binary packages were created with/without "Rules-Require-Root: no". Then they were compared using diffoscope, showing no outputs. > 16) typo-in-manual-page: I reported this issue to the upstream and sent a pull request as well: https://github.com/libyal/libewf-legacy/pull/19 It looks like the maintainer made a similar change and the issue has been addressed: https://github.com/libyal/libewf-legacy/commit/cd360ab8c9942dc5fbd35c9b3c08de6c1b653f16#diff-c0837cc425f249eba2a50ae06ff97acde680a718801aeb6d6114727bfb85cf1d https://github.com/libyal/libewf-legacy/commit/cd360ab8c9942dc5fbd35c9b3c08de6c1b653f16#diff-89a98a96ecbdd593f35c3728666b86eaeebba5b6f8b5013891c9d28d322f46e9 Note that typo-in-manual-page is still in the lintian report as the latest upstream source has not been incorporated. On top of these issues, I also addressed package-does-not-install-examples: https://salsa.debian.org/dfukui/libewf/-/jobs/3079374#L41 See the comparison result for details of how it was addressed: https://salsa.debian.org/dfukui/libewf/-/compare/debian%2F20140813-1-old...debian%2Fmaster?from_project_id=69076 > I've tried to do a more thorough review as you seem interested in > learning all of it. Please let me know if you'd like me to only > mention what is required (which would be totally fine as you're > contributing and whatever you do is still good for Debian). Thanks as usual for taking time. Yes, I enjoy learning much about packaging, so I would appreciate it if you give me thorough feedback as you've done so far. > On a high level, I suggest always making sure lintian gets run on your > build process, preferably with the following parameters: '-i', '-I', > '-E', '--pedantic'. > You don't need to solve all lintian findings, some of them might not > make much sense or just not be doable at all, but it's good to try to > understand at least what's critical so you can make a judgement call. Thanks also for pointing this out. I remember you made a similar comment for a different package - unhide. I'm embarrassed to say that I was working on unhide before reading your reply on libewf and suggestion as above. Sorry for making you repeat the same thing. Yeah, I will try to add the parameters to my build process (debuild) and, if possible, use a salsa CI job as well. Best, Fukui On Mon, 1 Aug 2022 at 04:33, Samuel Henrique <[email protected]> wrote: > Hello Daichi, I'm dropping Sebastian from the thread as I don't think > he's interested in this, > > >> Thanks a lot for the feedback. > >> Plus, apologies for my very late reply. > > No worries about the delay, as you can notice, I might also take a bit > to reply back so it's all fine. > > >> > 1) d/changelog: > >> That's right. > >> It' just that I forgot to remove the Closes stanza from my draft. > > I removed that stanza from the d/changelog draft. > > Nice, I have a few comments about d/changelog in general so I'll write it > here: > 4) debian-changelog-line-too-long: > Lintian is complaining about line number 9 being too long, so that > line needs to be broken into two. > > 5) no-nmu-in-changelog and source-nmu-has-incorrect-version-number: > Lintian thinks this is an NMU upload because there's no "* Team > upload" line in the changelog. Please add it as the very first line in > the changelog entries (it doesn't need to be under anyone's name, in > other words, this can be added to line number 3). > > 6) spelling-error-in-changelog Debian Debian (duplicate word): > This is a false-positive from lintian as it doesn't properly parse the > "Debian Janitor" entry, this will go away when Lintian addresses the > issue or a new changelog entry is made. You can ignore it, but if you > want to perform a workaround for it, you can swap line 4 with line 5 > (breaking the "debian... debian" chain in the changelog). > > >> > 2) d/tests: > >> It seems very useful to use autopkgtest-pkg-python for this kind of > testing. > >> Yes, I'll update d/control as you suggested and see it's enough for our > test case. > >> Thanks for introducing that package. > > autopkgtest-pkg-python was added to d/control. > > An autopkgtest job successfully passed in CI: > > https://salsa.debian.org/dfukui/libewf/-/pipelines/398401 > > Perfect. > > >> > 3) d/copyright: > >> licensecheck reports a bunch of source files that are subject to other > licenses than LGPL3+. > >> It may need much effort to review that license scan report and update > d/copyright accordingly. > >> So I'd appreciate it if you allow me to take some time. > > New files, copyright holders, and licenses were added to d/copyright. > > To review how things are changed, take a look at the following commit: > > > https://salsa.debian.org/dfukui/libewf/-/commit/c2fcbfd76ec8705765fa8e0e6bc85beade3a4cfe > > Cool, a few problems arose from those changes, most of these are > coming from Lintian, double check if you can see them in your dev > environment too: > > 7) bad-exception-format-in-dep5-copyright expat with public-domain: > We usually use the "foo with bar" pattern to show that there's license > exception in place, which is not the case here, the code is instead > slightly dual licensed (the FSF contributions are public-domain). > I suggest changing the license name to "Expat and public-domain". > > 8) invalid-short-name-in-dep5-copyright: > There are a couple of licenses with the wrong name in d/copyright, you > can read the lintian's output to find all of them. The lintian's > finding description also points to the documentation which has a table > of known license names. > Let me know if you have trouble finding any of that and I will help you. > > 9) license-file-listed-in-debian-copyright COPYING: > As per the finding's full description, this can be removed. > > 10) Files: debian/*: > You can add your name to this entry, and also the names of anybody > else who contributed to the package. I'll leave it up to you to decide > whether or not to do it as I don't consider this critical and I know > d/copyright stuff can be too time consuming. > The lintian finding "update-debian-copyright 2015 vs 2022" is about this. > > 11) Files: manuals/ewfinfo.1 ...: > The whole entry that lists the manuals/* files can be removed as they > all match under the "Files: *" entry and have the same license > details. > > 12) unused-license-paragraph-in-dep5-copyright gpl3+ > [debian/copyright:133]: > There's no file with "License: GPL3+", so the license paragraph can be > removed. You can read the lintian finding in its entirety if you need > more details. > > 13) Copyright: Copyright (C): > You don't need to add "Copyright (C)" to the "Copyright" field of > d/copyright, that can be removed. > > > Meanwhile, a new tag debian/20140813-1 was also added: > > https://salsa.debian.org/dfukui/libewf/-/tags/debian%2F20140813-1 > > Great, the debian/ tag is not generally needed by the way, as the > sponsor is the one who should push that ideally, but I don't mind if > you push it. I know gbp will work smoother if you create a debian/ tag > (it lets you do gbp push without extra parameters if there's a tag). > > Some extra notes now: > > 12) older-debian-watch-file-standard 3: > You can bump d/watch to version 4. > > 13) out-of-date-standards-version 4.5.0: > You can bump Standards-Version to the latest one if you go through the > upgrading checklist and consider the package is compliant with the > latest version: > https://www.debian.org/doc/debian-policy/upgrading-checklist.html > > 14) silent-on-rules-requiring-root: > You can check if Rules-require-root is needed and set the value > accordingly in d/control. The way I like to check is by performing two > builds, one with each value, and then comparing the binaries with > diffoscope, if they're the same, then root is not needed. > > 15) debian-rules-uses-as-needed-linker-flag: > That flag can be removed from d/rules, as indicated by lintian. > > 16) typo-in-manual-page: > Lintian is detecting a few typos, this might be a good opportunity for > you to send patches upstream fixing those, but this is definitely not > required for the upload to be made. > > I've tried to do a more thorough review as you seem interested in > learning all of it. Please let me know if you'd like me to only > mention what is required (which would be totally fine as you're > contributing and whatever you do is still good for Debian). > > On a high level, I suggest always making sure lintian gets run on your > build process, preferably with the following parameters: '-i', '-I', > '-E', '--pedantic'. > You don't need to solve all lintian findings, some of them might not > make much sense or just not be doable at all, but it's good to try to > understand at least what's critical so you can make a judgement call. > > Let me know if you'd like help on any of this, > > Thanks for contributing, > > -- > Samuel Henrique <samueloph> >
