Re: DD ping [Brutespray]
Hello Stéphane, > I followed your recommendations and modified the control file. It's cleaner > that way. > (Sorry for the last two commits, it's ugly, I should have been careful and > pushed this in one go) Don't worry, I believe the work we do on Debian is the best way to learn real world git and you will always find something you can improve while at it. > In fact, since several people have been involved in maintaining this package > (including those helping me), > I wonder if my name is really legitimate in the "Uploaders" field of the > d/control file. Well, nobody's gonna say it's not legitimate, that's for sure, the thing is that we always need to have at least one person in the Uploaders field, this person being the first level contact for package issues, the team being the second. For me having my name in the Uploaders field is more about easily spotting things that needs to be made (bugs, new releases...) as I'm unfortunately currently spending most time in those packages other than the ones from the team. Our team does not have strong ownership enforcement so you are definitely encouraged to fix things from other packages and you should put your name as an Uploader if you're interested in it (it's a good idea to contact the current Uploaders through this list first). I noticed the tags "debian/1.6.8-1" and "debian/1.6.7-1" were pushed to the git repo, I removed both and pointed "debian/1.6.8-1" at the right commit, don't forget to remove these tags from your local clone and update from the remote. As a rule of thumb, the person who does the upload of the package is the one who should push the "debian/" tags, preferably signed by their gpg key (which I should/will start doing soon). > Thank you for your patience and your precious help Samuel. You're welcome, I'm happy to help. Package uploaded, keep up the good work :) -- Samuel Henrique
Re: DD ping [Brutespray]
Hello Samuel, > > Upstream was kind enough to push another release 1.6.8 so I wasn't sure > if I had to keep version 1.6.7 in the changelog file. > > I finally decided to update the above fields for version 1.6.8, but I'm > not sure so tell me if I was wrong. > > You did the right thing, we only need to keep the changelog entry if > an upload was made. Some people like to keep the release field of the > changelog as "UNRELEASED" until right before the upload to denote > that, while I rather check the tag to see if there was an upload as > changing to UNRELEASED->unstable would require an extra commit just > for that. > I understand, I'll keep that in mind for the next time. > > > I can see another lintian warning "changelog-should-not-mention-nmu", > should I remove the Uploaders line in d/control ? > > I see that you solved that by changing your name in d/changelog but I > do believe you want to change the entry in d/control instead. Just > consider that you can chose how your name/email is written and > d/control, you also control how it gets automatically written to > d/changelog[0] and you need to make those two matches because > otherwise our tooling will not recognize the upload as being made from > either the maintainer or the uploader (in our case it would need to be > marked as team upload). Now, with this in mind, consider which is the > way you want to have that written and put your tooling[0] and > d/control in sync and you will never again have this issue. The > developers reference has some good explanations about NMU and Team > Uploads in case you're not aware of it yet[1]. > I followed your recommendations and modified the control file. It's cleaner that way. (Sorry for the last two commits, it's ugly, I should have been careful and pushed this in one go) In fact, since several people have been involved in maintaining this package (including those helping me), I wonder if my name is really legitimate in the "Uploaders" field of the d/control file. > > I also added a quick test (brutespray -h) > > That's a nice addition, I'd like to ask you to add the restriction > "superficial" to it, every test which uses only "-h" should have this > restriction, and if you find one without it, feel free to change it. > The "skippable" restriction can be removed in favor of "superficial". > For more info, you can refer to the docs at [2]. > Ok thanks, it's done. I will take time to think about doing a more serious test for the next release. This one is not really testing much, actually. > > The rest of the package is all fine, so as soon as you update the test > (and change your name in d/control, if you decide to follow that way) > I will do the upload. > > Thanks for your work :) > Thank you for your patience and your precious help Samuel.
Re: DD ping [Brutespray]
Hello Stéphane, > Upstream was kind enough to push another release 1.6.8 so I wasn't sure if I > had to keep version 1.6.7 in the changelog file. > I finally decided to update the above fields for version 1.6.8, but I'm not > sure so tell me if I was wrong. You did the right thing, we only need to keep the changelog entry if an upload was made. Some people like to keep the release field of the changelog as "UNRELEASED" until right before the upload to denote that, while I rather check the tag to see if there was an upload as changing to UNRELEASED->unstable would require an extra commit just for that. > Thank you for all these clarifications. I think this new v1.6.8 fix now that > ambiguity properly. That's correct, and it was fixed in the best possible way. > I can see another lintian warning "changelog-should-not-mention-nmu", should > I remove the Uploaders line in d/control ? I see that you solved that by changing your name in d/changelog but I do believe you want to change the entry in d/control instead. Just consider that you can chose how your name/email is written and d/control, you also control how it gets automatically written to d/changelog[0] and you need to make those two matches because otherwise our tooling will not recognize the upload as being made from either the maintainer or the uploader (in our case it would need to be marked as team upload). Now, with this in mind, consider which is the way you want to have that written and put your tooling[0] and d/control in sync and you will never again have this issue. The developers reference has some good explanations about NMU and Team Uploads in case you're not aware of it yet[1]. > I also added a quick test (brutespray -h) That's a nice addition, I'd like to ask you to add the restriction "superficial" to it, every test which uses only "-h" should have this restriction, and if you find one without it, feel free to change it. The "skippable" restriction can be removed in favor of "superficial". For more info, you can refer to the docs at [2]. The rest of the package is all fine, so as soon as you update the test (and change your name in d/control, if you decide to follow that way) I will do the upload. Thanks for your work :) [0] You can add this to your bashrc and gbp/dch will pick it up: DEBEMAIL="your email address" DEBFULLNAME="your name" export DEBEMAIL DEBFULLNAME [1] https://www.debian.org/doc/manuals/developers-reference/pkgs.html#non-maintainer-uploads-nmus [2] https://salsa.debian.org/ci-team/autopkgtest/raw/master/doc/README.package-tests.rst -- Samuel Henrique
Re: DD ping [Brutespray]
Hello Samuel, Your changes are fine, but there are two issues now: > 1) I believe you forgot to update d/changelog with the new changes, > you need to add: > * Bump to standards-version version 4.5.0 > * Add Rules-Requires-Root: no > * Update Uploaders field to match changelog > > Sorry about that. Upstream was kind enough to push another release 1.6.8 so I wasn't sure if I had to keep version 1.6.7 in the changelog file. I finally decided to update the above fields for version 1.6.8, but I'm not sure so tell me if I was wrong. 2) If I understood correctly, upstream just re-released 1.6.7, which > should never ever happen as it causes confusion as there is now two > 1.6.7 releases of brutespray (even if upstream hides the first one). > The package is currently not building with gbp as the files are > differing, you can see that the pristine-tar branch is not updated and > the tag brutespray-1.6.7 points to the first 1.6.7 release. > > Since this happened, I suggest packaging a git snapshot of the > repository, so you can use something like 1.6.7+git20200423.fd9a370-1 > and/or suggest upstream to release 1.6.8 since there is ambiguity over > 1.6.7, but it could be that only Debian got affected by it. > Another alternative would be to fix the pristine-tar and upstream > branches to be in sync with the 1.6.7 you want to use, but that might > be tricky and in the end I wouldn't be able to see from the version > number which of the 1.6.7 releases are you using. > > That being said, I do understand that the diff between both 1.6.7 > versions is very small, and doesn't affect the functionality of > brutespray, but now that you imported both to the repo, the issue > needs to be solved. > > Again, my suggestion is to go for the easiest approach to package a > git snapshot, as it will make it clear which commit you're using. > > What are your thoughts on this? > Thank you for all these clarifications. I think this new v1.6.8 fix now that ambiguity properly. I can see another lintian warning "changelog-should-not-mention-nmu", should I remove the Uploaders line in d/control ? I also added a quick test (brutespray -h) Best regards, Stephane
Re: DD ping [Brutespray]
Hello Stéphane, Your changes are fine, but there are two issues now: 1) I believe you forgot to update d/changelog with the new changes, you need to add: * Bump to standards-version version 4.5.0 * Add Rules-Requires-Root: no * Update Uploaders field to match changelog 2) If I understood correctly, upstream just re-released 1.6.7, which should never ever happen as it causes confusion as there is now two 1.6.7 releases of brutespray (even if upstream hides the first one). The package is currently not building with gbp as the files are differing, you can see that the pristine-tar branch is not updated and the tag brutespray-1.6.7 points to the first 1.6.7 release. Since this happened, I suggest packaging a git snapshot of the repository, so you can use something like 1.6.7+git20200423.fd9a370-1 and/or suggest upstream to release 1.6.8 since there is ambiguity over 1.6.7, but it could be that only Debian got affected by it. Another alternative would be to fix the pristine-tar and upstream branches to be in sync with the 1.6.7 you want to use, but that might be tricky and in the end I wouldn't be able to see from the version number which of the 1.6.7 releases are you using. That being said, I do understand that the diff between both 1.6.7 versions is very small, and doesn't affect the functionality of brutespray, but now that you imported both to the repo, the issue needs to be solved. Again, my suggestion is to go for the easiest approach to package a git snapshot, as it will make it clear which commit you're using. What are your thoughts on this? Also, as a fun fact, the first ever official release of Debian was 1.1 because the project got bitten by the same issue, although caused by a third party: https://lists.debian.org/debian-announce/1995/msg00010.html Regards, -- Samuel Henrique
Re: DD ping [Brutespray]
Hello Samuel, > > > > 2) "P: brutespray source: rules-requires-root-missing" Lintian is > > > complaining about this[0], I know it's pedantic but it's nice to have, > > > tell me if you have questions on how to address it. > > > > Thanks, If you could show me how to use diffoscope properly, to make > > sure everything's okay, I wouldn't say no... :) > > Alright, so it's fairly simple, diffoscope is available under the > diffoscope package, and what you need to do is build the package > twice: one time with the package as it is, and one time with the > "rules require root" change, then you run diffoscope against the two > different .debs that you produced, if the output is empty, then you > know that the change didn't affect the binaries and thus it's fine. > > If there are any differences, then you know you can't apply the change > and you don't need to investigate why, but this case is very rare from > what I've seen. Thank you for your explanations Samuel. I've just tested and I don't actually see any difference between both generated packages, so it seems ok to me. I've also updated the CHANGELOG.md file which now correctly lists version 1.6.7 according to upstream (https://github.com/x90skysn3k/brutespray/issues/39) Best regards, Stephane
Re: DD ping [Brutespray]
Hello Stéphane, > > 2) "P: brutespray source: rules-requires-root-missing" Lintian is > > complaining about this[0], I know it's pedantic but it's nice to have, > > tell me if you have questions on how to address it. > > Thanks, If you could show me how to use diffoscope properly, to make > sure everything's okay, I wouldn't say no... :) Alright, so it's fairly simple, diffoscope is available under the diffoscope package, and what you need to do is build the package twice: one time with the package as it is, and one time with the "rules require root" change, then you run diffoscope against the two different .debs that you produced, if the output is empty, then you know that the change didn't affect the binaries and thus it's fine. If there are any differences, then you know you can't apply the change and you don't need to investigate why, but this case is very rare from what I've seen. Regards, -- Samuel Henrique
Re: DD ping [Brutespray]
Hi Samuel, > I removed some cruft from the changelog and then noticed an issue, so > can you take a look at the following? > > 1) Your name in d/control is not matching what is in d/changelog, you > should fix that and Lintian should not complain about: "brutespray > source: changelog-should-mention-nmu" Sure, I've updated the control file. > > 2) "P: brutespray source: rules-requires-root-missing" Lintian is > complaining about this[0], I know it's pedantic but it's nice to have, > tell me if you have questions on how to address it. Thanks, If you could show me how to use diffoscope properly, to make sure everything's okay, I wouldn't say no... :) > > 3) "I: brutespray source: out-of-date-standards-version 4.4.1 > (released 2019-09-29) (current is 4.5.0)" Please take a look at the > Debian policy upgrade checklist and bump the version to 4.5.0. Done ! > > Thanks for your work on the package! Thanks for your help Samuel !
Re: DD ping [Brutespray]
Hello Stéphane, > I've made a new release of brutespray (debian/1.6.7-1 ). > Could you please review it ? I removed some cruft from the changelog and then noticed an issue, so can you take a look at the following? 1) Your name in d/control is not matching what is in d/changelog, you should fix that and Lintian should not complain about: "brutespray source: changelog-should-mention-nmu" 2) "P: brutespray source: rules-requires-root-missing" Lintian is complaining about this[0], I know it's pedantic but it's nice to have, tell me if you have questions on how to address it. 3) "I: brutespray source: out-of-date-standards-version 4.4.1 (released 2019-09-29) (current is 4.5.0)" Please take a look at the Debian policy upgrade checklist and bump the version to 4.5.0. Thanks for your work on the package! [0] https://lintian.debian.org/tags/rules-requires-root-missing.html -- Samuel Henrique
DD ping [Brutespray]
Hi team! I've made a new release of brutespray (debian/1.6.7-1 ). Could you please review it ? https://salsa.debian.org/pkg-security-team/brutespray Thanks ! Stéphane
