Re: Request to review and upload libewf 20140813-1
Hello Daichi, Sorry for the long delay, I was on vacation and should probably have mentioned it on this list, but now I'm back. > > 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. That is looking weird indeed, let's leave it like that as I consider it fine. > > 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. Perfect, that's the best way to check. > > 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. That's ok. > 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 Great. > > 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. Cool, just in case you're looking for something, the unhide package has a bug report about the inclusion of the GUI, I missed the need to add Breaks+Replaces as the package migrated to testing: https://bugs.debian.org/1016613 > > 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. Don't worry about it, it's fine and I don't mind repeating it if needed, I know both occasions happened at roughly the same time as well so I thought that was the case too. > Yeah, I will try to add the parameters to my build process (debuild) and, if > possible, use a salsa CI job as well. I have sponsored the upload for you, thanks a lot for contributing! Note that I had to rebase your branch on top of the team's repo since I had merged a change from Janitor, this means you will have to reset your fork's debian/master branch if you plan on using it further (or just delete the fork and recreate it when needed in the future). Regards, -- Samuel Henrique
Re: Request to review and upload libewf 20140813-1
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 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
Re: Request to review and upload libewf 20140813-1
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
Re: Request to review and upload libewf 20140813-1
Hello Samuel, I would like to send an updated source package of libewf. Kindly find my inline comments below. On Fri, 1 Jul 2022 at 21:51, Daichi Fukui wrote: > Hello Samuel, > > Thanks a lot for the feedback. > Plus, apologies for my very late reply. > > With this email, I just wanted to say I've just started working on further > improvement based on your recent comments. > Though I'm trying to fix flaws on my former proposed package, I would like > to address the issues you mentioned, as follows. > > > 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. > 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 > > 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 Meanwhile, a new tag debian/20140813-1 was also added: https://salsa.debian.org/dfukui/libewf/-/tags/debian%2F20140813-1 I would appreciate it if you consider uploading this source package. When the improvement is satisfactory, I'll share an updated source package > with you. > > Best, > Fukui > Best, Fukui
Re: Request to review and upload libewf 20140813-1
I found a serious typo. > Yes, I'll update d/control as you suggested and see it's enough for our > test case. > I'll update d/tests/control, rather. Best, Fukui
Re: Request to review and upload libewf 20140813-1
Hello Samuel, Thanks a lot for the feedback. Plus, apologies for my very late reply. With this email, I just wanted to say I've just started working on further improvement based on your recent comments. Though I'm trying to fix flaws on my former proposed package, I would like to address the issues you mentioned, as follows. > 1) d/changelog: That's right. It' just that I forgot to remove the Closes stanza from my draft. > 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. > 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. When the improvement is satisfactory, I'll share an updated source package with you. Best, Fukui
Re: Request to review and upload libewf 20140813-1
Hello Daichi, As Sebastian already explained the situation with the openssl transition, I'll leave that out of the review. I'll enumerate things to make discussions easier as we can name them if needed: 1) d/changelog: The "Closes" stanza should not be present in that changelog entry because that bug has already been closed on the previous upload. 2) d/tests: I see that the test present in there is basically the same thing that autopkgtest-pkg-python does, so I suggest using that. It's possible that you've tried using it and had issues due to the test trying to import the wrong module, in order to fix that you'll need to do what's described in here: https://manpages.debian.org/testing/autodep8/autodep8.1.en.html#python_(debian/tests/autopkgtest-pkg-python.conf) And just in case you're not familiar with "autopkgtest-pkg-python", here's some guidance: https://wiki.debian.org/Python/LibraryStyleGuide#autopkgtest Hopefully that will make things simpler. 3) d/copyright: It's a good idea to improve the copyright entry, as you can see upstream bumped the years if you look at the diff. 2006-2021, Joachim Metz The entry for "Files: *" can be superseded by this single entry (it's outdated since a few uploads). The copyright file is a bit outdated in general, so it would be better to recheck everything, but it's ok if you only want to change this part. Everything else looks fine, we can proceed Thank you for your contributions! -- Samuel Henrique
Re: Request to review and upload libewf 20140813-1
On 2022-06-10 23:23:35 +0900, Daichi Fukui wrote: > Hello team, > (CC: Samuel. Sebastian) > > I've prepared a new version of libewf [0], which is going to be > 20140813-1 with this update. > This version mainly introduces the following changes: > > * New upstream version 20140813 (Closes: #1006393) > * Switch to debhelper compat level 13 > * Update symbols file > * Add autopkgtest > > Additionally, since the source code for debian/20140807-2.1 is > currently missing in salsa, that source code is also included in this > update [1]. > > This new source package was built and tested using salsa-ci, and > everything but test-crossbuild-arm64 [2] successfully passed. > I will keep that failing job untouched because it is allowed to fail > as you can see [3]. > That said, if we have to address this issue before uploading > 20140813-1, please let me know. > > If this update is satisfactory and helpful, I would appreciate it if > you review and upload the package. > > By the way, one thing I'm worried about is the migration status of > this package. For some reasons, its migration is blocked by openssl > and that keeps the issue #1006393 unresolved, which would result in > the removal of libewf from testing on June 21st [4]. > If I understand correctly, according to developer information [5], we > should "avoid uploads unrelated to this transition" probably until > issues of openssl are resolved. If this guidance applies to this draft > source package, we will have to suspend this draft and wait for issues > of openssl being resolved. That's expected. The package is involved in the still ongoing openssl transition. As an upload was required to make it build with openssl 3, that upload was blocked behind openssl 3. As both have now migrated to testing, the warning should be gone from libewf's tracker. Cheers > > Hope this makes sense. > > [0] https://salsa.debian.org/dfukui/libewf/-/commits/debian/master > Tag: https://salsa.debian.org/dfukui/libewf/-/tags/debian%2F20140813-1 > [1] > https://salsa.debian.org/dfukui/libewf/-/commit/00c9537a456d56f92f2582133dfdb456314cd785 > [2] https://salsa.debian.org/dfukui/libewf/-/pipelines/387710 > [3] > https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/pipeline-jobs.yml > [4] https://qa.debian.org/excuses.php?package=libewf > [5] https://tracker.debian.org/pkg/libewf > > Best regards, > Fukui -- Sebastian Ramacher
Request to review and upload libewf 20140813-1
Hello team, (CC: Samuel. Sebastian) I've prepared a new version of libewf [0], which is going to be 20140813-1 with this update. This version mainly introduces the following changes: * New upstream version 20140813 (Closes: #1006393) * Switch to debhelper compat level 13 * Update symbols file * Add autopkgtest Additionally, since the source code for debian/20140807-2.1 is currently missing in salsa, that source code is also included in this update [1]. This new source package was built and tested using salsa-ci, and everything but test-crossbuild-arm64 [2] successfully passed. I will keep that failing job untouched because it is allowed to fail as you can see [3]. That said, if we have to address this issue before uploading 20140813-1, please let me know. If this update is satisfactory and helpful, I would appreciate it if you review and upload the package. By the way, one thing I'm worried about is the migration status of this package. For some reasons, its migration is blocked by openssl and that keeps the issue #1006393 unresolved, which would result in the removal of libewf from testing on June 21st [4]. If I understand correctly, according to developer information [5], we should "avoid uploads unrelated to this transition" probably until issues of openssl are resolved. If this guidance applies to this draft source package, we will have to suspend this draft and wait for issues of openssl being resolved. Hope this makes sense. [0] https://salsa.debian.org/dfukui/libewf/-/commits/debian/master Tag: https://salsa.debian.org/dfukui/libewf/-/tags/debian%2F20140813-1 [1] https://salsa.debian.org/dfukui/libewf/-/commit/00c9537a456d56f92f2582133dfdb456314cd785 [2] https://salsa.debian.org/dfukui/libewf/-/pipelines/387710 [3] https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/pipeline-jobs.yml [4] https://qa.debian.org/excuses.php?package=libewf [5] https://tracker.debian.org/pkg/libewf Best regards, Fukui
