Bug#890594: salsa script ready to review
Le 27/11/2018 à 15:13, Raphael Hertzog a écrit : > On Tue, 27 Nov 2018, Xavier wrote: >> It seems that $_ is modified sometimes somewhere else. I fixed that in >> last upload (.deb updated also) > > Now it works fine. At least, I used it to ensure consistency across all > 223 repositories of the pkg-security team. > > Thank you for your work on this! @mattia: so seems ready for your review ;-) Thanks Raphaël for your time and debug !
Bug#890594: salsa script ready to review
On Tue, 27 Nov 2018, Xavier wrote: > It seems that $_ is modified sometimes somewhere else. I fixed that in > last upload (.deb updated also) Now it works fine. At least, I used it to ensure consistency across all 223 repositories of the pkg-security team. Thank you for your work on this! -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 27/11/2018 à 14:23, Raphael Hertzog a écrit : > Hi, > > On Mon, 26 Nov 2018, Xavier wrote: >> Done: I added a "rename_branch" command: >> - rename_head: >>* tries to create new branch and ignores failures >>* move default_branch to new value >>* deletes old branch only if creation succeeds >> - rename_branch: >>* creates new_branch and deletes oldest. Fails on first error > > Still some problem remaining. The new helper script used below > is in https://salsa.debian.org/pkg-security-team/pkg-security-team > > $ ./bin/update-repos radare2-cutter > salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter > salsa info: pkg-security-team/radare2-cutter id is 29721 > radare2-cutter: > bad description: Packaging for cutter from radare project > Default branch is upstream > 1 packages misconfigured, update them ? (Y/n) > salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter > salsa info: Configuring radare2-cutter > salsa warn: Deleting old email-on-push (redirected to > dispa...@tracker.debian.org) > salsa info: Email-on-push hook added to project 29721 (recipients: > dispa...@tracker.debian.org) > salsa warn: Deleting old tagpending (was > https://webhook.salsa.debian.org/tagpending/radare2-cutter) > salsa info: Tagpending hook added to project 29721 > Not an ARRAY reference at /usr/share/perl5/Devscripts/Salsa/update_repo.pm > line 67, line 1. It seems that $_ is modified sometimes somewhere else. I fixed that in last upload (.deb updated also) > Cheers, Thanks!
Bug#890594: salsa script ready to review
Hi, On Mon, 26 Nov 2018, Xavier wrote: > Done: I added a "rename_branch" command: > - rename_head: >* tries to create new branch and ignores failures >* move default_branch to new value >* deletes old branch only if creation succeeds > - rename_branch: >* creates new_branch and deletes oldest. Fails on first error Still some problem remaining. The new helper script used below is in https://salsa.debian.org/pkg-security-team/pkg-security-team $ ./bin/update-repos radare2-cutter salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter salsa info: pkg-security-team/radare2-cutter id is 29721 radare2-cutter: bad description: Packaging for cutter from radare project Default branch is upstream 1 packages misconfigured, update them ? (Y/n) salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter salsa info: Configuring radare2-cutter salsa warn: Deleting old email-on-push (redirected to dispa...@tracker.debian.org) salsa info: Email-on-push hook added to project 29721 (recipients: dispa...@tracker.debian.org) salsa warn: Deleting old tagpending (was https://webhook.salsa.debian.org/tagpending/radare2-cutter) salsa info: Tagpending hook added to project 29721 Not an ARRAY reference at /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 67, line 1. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 26/11/2018 à 07:04, Xavier a écrit : > Le 25/11/2018 à 23:25, Raphael Hertzog a écrit : >> On Sun, 25 Nov 2018, Xavier wrote: >>> My last change produces this bug. Fixed now, I pushed a new .deb in the >>> same place. >> >> Next try with a package whose HEAD points to the wrong branch (upstream) >> but that already has the desired target branch (debian/master) >> >> $ salsa --conf-file +./salsa-pkg-security.conf update_safe radare2-cutter >> --verbose --debug >> salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter >> salsa info: pkg-security-team/radare2-cutter id is 29721 >> radare2-cutter: >> bad description: Packaging for cutter from radare project >> Default branch is upstream >> 1 packages misconfigured, update them ? (Y/n) >> salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter >> salsa info: Configuring radare2-cutter >> salsa warn: Error POSTing >> https://salsa.debian.org/api/v4/projects/29721/repository/branches (HTTP >> 400): Bad Request {"message":"Branch already exists"} at >> /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. >> >> Now a well configured repository but I inversed SOURCE_BRANCH and >> DEST_BRANCH. The operation >> seems to work: >> $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose >> --debug >> salsa info: Project acct => pkg-security-team/acct >> salsa info: pkg-security-team/acct id is 6641 >> acct: >> Default branch is debian/master >> 1 packages misconfigured, update them ? (Y/n) >> salsa info: Project acct => pkg-security-team/acct >> salsa info: Configuring acct >> salsa warn: Deleting old email-on-push (redirected to >> dispa...@tracker.debian.org) >> salsa info: Email-on-push hook added to project 6641 (recipients: >> dispa...@tracker.debian.org) >> salsa warn: Deleting old tagpending (was >> https://webhook.salsa.debian.org/tagpending/acct) >> salsa info: Tagpending hook added to project 6641 >> salsa info: Project 6641 updated >> >> Now I put SOURCE_BRANCH and DEST_BRANCH to their desired value and the next >> run doesn't work: >> $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose >> --debug >> salsa info: Project acct => pkg-security-team/acct >> salsa info: pkg-security-team/acct id is 6641 >> acct: >> Default branch is master >> 1 packages misconfigured, update them ? (Y/n) >> salsa info: Project acct => pkg-security-team/acct >> salsa info: Configuring acct >> salsa warn: Error POSTing >> https://salsa.debian.org/api/v4/projects/6641/repository/branches (HTTP >> 400): Bad Request {"message":"Branch already exists"} at >> /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. >> >> Maybe you should separate the "rename branch" operation from the "set HEAD >> to" >> operation. Because we want to fix HEAD pointing to the wrong branch even when >> there's nothing to rename. And it could be useful for example to rename all >> "upstream" into "upstream/latest" for example (without changing HEAD). >> >> SALSA_RENAME_BRANCH="upstream:upstream/latest master:debian/master" >> SALSA_SET_HEAD="debian/master" >> >> Cheers, > > Hello, > > half fixed: > - "rename head" is tried even if branch already exists > - original branch is deleted only if new branch creation was successful > - I'll write a separate "rename" command to rename another branch. > - .deb updated > > Cheers, Done: I added a "rename_branch" command: - rename_head: * tries to create new branch and ignores failures * move default_branch to new value * deletes old branch only if creation succeeds - rename_branch: * creates new_branch and deletes oldest. Fails on first error If '--no-fail' is used, both continues with next repo. New .deb is building, I'll push it at ~12:00 GMT+1
Bug#890594: salsa script ready to review
Le 25/11/2018 à 23:25, Raphael Hertzog a écrit : > On Sun, 25 Nov 2018, Xavier wrote: >> My last change produces this bug. Fixed now, I pushed a new .deb in the >> same place. > > Next try with a package whose HEAD points to the wrong branch (upstream) > but that already has the desired target branch (debian/master) > > $ salsa --conf-file +./salsa-pkg-security.conf update_safe radare2-cutter > --verbose --debug > salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter > salsa info: pkg-security-team/radare2-cutter id is 29721 > radare2-cutter: > bad description: Packaging for cutter from radare project > Default branch is upstream > 1 packages misconfigured, update them ? (Y/n) > salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter > salsa info: Configuring radare2-cutter > salsa warn: Error POSTing > https://salsa.debian.org/api/v4/projects/29721/repository/branches (HTTP > 400): Bad Request {"message":"Branch already exists"} at > /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. > > Now a well configured repository but I inversed SOURCE_BRANCH and > DEST_BRANCH. The operation > seems to work: > $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose > --debug > salsa info: Project acct => pkg-security-team/acct > salsa info: pkg-security-team/acct id is 6641 > acct: > Default branch is debian/master > 1 packages misconfigured, update them ? (Y/n) > salsa info: Project acct => pkg-security-team/acct > salsa info: Configuring acct > salsa warn: Deleting old email-on-push (redirected to > dispa...@tracker.debian.org) > salsa info: Email-on-push hook added to project 6641 (recipients: > dispa...@tracker.debian.org) > salsa warn: Deleting old tagpending (was > https://webhook.salsa.debian.org/tagpending/acct) > salsa info: Tagpending hook added to project 6641 > salsa info: Project 6641 updated > > Now I put SOURCE_BRANCH and DEST_BRANCH to their desired value and the next > run doesn't work: > $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose > --debug > salsa info: Project acct => pkg-security-team/acct > salsa info: pkg-security-team/acct id is 6641 > acct: > Default branch is master > 1 packages misconfigured, update them ? (Y/n) > salsa info: Project acct => pkg-security-team/acct > salsa info: Configuring acct > salsa warn: Error POSTing > https://salsa.debian.org/api/v4/projects/6641/repository/branches (HTTP 400): > Bad Request {"message":"Branch already exists"} at > /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. > > Maybe you should separate the "rename branch" operation from the "set HEAD to" > operation. Because we want to fix HEAD pointing to the wrong branch even when > there's nothing to rename. And it could be useful for example to rename all > "upstream" into "upstream/latest" for example (without changing HEAD). > > SALSA_RENAME_BRANCH="upstream:upstream/latest master:debian/master" > SALSA_SET_HEAD="debian/master" > > Cheers, Hello, half fixed: - "rename head" is tried even if branch already exists - original branch is deleted only if new branch creation was successful - I'll write a separate "rename" command to rename another branch. - .deb updated Cheers,
Bug#890594: salsa script ready to review
On Sun, 25 Nov 2018, Xavier wrote: > My last change produces this bug. Fixed now, I pushed a new .deb in the > same place. Next try with a package whose HEAD points to the wrong branch (upstream) but that already has the desired target branch (debian/master) $ salsa --conf-file +./salsa-pkg-security.conf update_safe radare2-cutter --verbose --debug salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter salsa info: pkg-security-team/radare2-cutter id is 29721 radare2-cutter: bad description: Packaging for cutter from radare project Default branch is upstream 1 packages misconfigured, update them ? (Y/n) salsa info: Project radare2-cutter => pkg-security-team/radare2-cutter salsa info: Configuring radare2-cutter salsa warn: Error POSTing https://salsa.debian.org/api/v4/projects/29721/repository/branches (HTTP 400): Bad Request {"message":"Branch already exists"} at /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. Now a well configured repository but I inversed SOURCE_BRANCH and DEST_BRANCH. The operation seems to work: $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose --debug salsa info: Project acct => pkg-security-team/acct salsa info: pkg-security-team/acct id is 6641 acct: Default branch is debian/master 1 packages misconfigured, update them ? (Y/n) salsa info: Project acct => pkg-security-team/acct salsa info: Configuring acct salsa warn: Deleting old email-on-push (redirected to dispa...@tracker.debian.org) salsa info: Email-on-push hook added to project 6641 (recipients: dispa...@tracker.debian.org) salsa warn: Deleting old tagpending (was https://webhook.salsa.debian.org/tagpending/acct) salsa info: Tagpending hook added to project 6641 salsa info: Project 6641 updated Now I put SOURCE_BRANCH and DEST_BRANCH to their desired value and the next run doesn't work: $ salsa --conf-file +./salsa-pkg-security.conf update_safe acct --verbose --debug salsa info: Project acct => pkg-security-team/acct salsa info: pkg-security-team/acct id is 6641 acct: Default branch is master 1 packages misconfigured, update them ? (Y/n) salsa info: Project acct => pkg-security-team/acct salsa info: Configuring acct salsa warn: Error POSTing https://salsa.debian.org/api/v4/projects/6641/repository/branches (HTTP 400): Bad Request {"message":"Branch already exists"} at /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 51. Maybe you should separate the "rename branch" operation from the "set HEAD to" operation. Because we want to fix HEAD pointing to the wrong branch even when there's nothing to rename. And it could be useful for example to rename all "upstream" into "upstream/latest" for example (without changing HEAD). SALSA_RENAME_BRANCH="upstream:upstream/latest master:debian/master" SALSA_SET_HEAD="debian/master" Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 25/11/2018 à 22:39, Xavier a écrit : > On Sun, 25 Nov 2018, Raphael Hertzog wrote: >> On Thu, 22 Nov 2018, Xavier wrote: >>> Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? >> >> I wanted to do some test but something else broke: >> >> $ salsa --conf-file +./salsa-pkg-security.conf update_safe sandsifter >> --verbose --debug >> salsa info: Project sandsifter => pkg-security-team/sandsifter >> salsa info: pkg-security-team/sandsifter id is 30154 >> sandsifter: >> bad description: x86 processor fuzzer >> 1 packages misconfigured, update them ? (Y/n) >> salsa info: Project sandsifter => pkg-security-team/sandsifter >> salsa info: Configuring sandsifter >> salsa warn: The #1 argument ($project_id) to project must be a scalar at >> /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 49. > > Here is the fix, I'm building a new .deb: > > diff --git a/lib/Devscripts/Salsa/update_repo.pm > b/lib/Devscripts/Salsa/update_repo.pm > index 316baec9..3d637a72 100644 > --- a/lib/Devscripts/Salsa/update_repo.pm > +++ b/lib/Devscripts/Salsa/update_repo.pm > @@ -46,7 +46,7 @@ sub _update_repo { > my $project; > # 1 - creates new branch if --rename-head > if ($self->config->rename_head) { > -my $project = $self->api->project($_[0]); > +my $project = $self->api->project($_->[0]); > if ($project->{default_branch} ne > $self->config->dest_branch) { > $self->api->create_branch( > $id, > @@ -57,7 +57,7 @@ sub _update_repo { > $configparams->{default_branch} >= $self->config->dest_branch; > } else { > -ds_verbose "Head already renamed for $_[1]"; > +ds_verbose "Head already renamed for $_->[1]"; > $project = undef; > } > } My last change produces this bug. Fixed now, I pushed a new .deb in the same place. Thanks for this report!
Bug#890594: salsa script ready to review
Le 25/11/2018 à 22:37, Raphael Hertzog a écrit : > On Sun, 25 Nov 2018, Raphael Hertzog wrote: >> On Thu, 22 Nov 2018, Xavier wrote: >>> Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? >> >> I wanted to do some test but something else broke: >> >> $ salsa --conf-file +./salsa-pkg-security.conf update_safe sandsifter >> --verbose --debug >> salsa info: Project sandsifter => pkg-security-team/sandsifter >> salsa info: pkg-security-team/sandsifter id is 30154 >> sandsifter: >> bad description: x86 processor fuzzer >> 1 packages misconfigured, update them ? (Y/n) >> salsa info: Project sandsifter => pkg-security-team/sandsifter >> salsa info: Configuring sandsifter >> salsa warn: The #1 argument ($project_id) to project must be a scalar at >> /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 49. > > Forgot to show my file: > $ cat salsa-pkg-security.conf > SALSA_GROUP=pkg-security-team > #SALSA_GROUP_ID=2586 > > # Skip the pkg-security-team project (containing this file) > SALSA_SKIP=pkg-security-team > > SALSA_DESC_PATTERN="%p packaging" > SALSA_DESC=yes > > SALSA_EMAIL=yes > SALSA_EMAIL_RECIPIENTS=dispa...@tracker.debian.org > > SALSA_IRKER=yes > SALSA_IRKER_HOST=ruprecht.snow-crash.org > SALSA_IRC_CHANNEL=debian-pkg-security > > SALSA_TAGPENDING=yes > > SALSA_SOURCE_BRANCH=master > SALSA_DEST_BRANCH=debian/master > SALSA_RENAME_HEAD=yes > > SALSA_ENABLE_MR=yes > SALSA_DISABLE_ISSUES=yes > > Cheers, > Here is the fix, I'm building a new .deb: diff --git a/lib/Devscripts/Salsa/update_repo.pm b/lib/Devscripts/Salsa/update_repo.pm index 316baec9..3d637a72 100644 --- a/lib/Devscripts/Salsa/update_repo.pm +++ b/lib/Devscripts/Salsa/update_repo.pm @@ -46,7 +46,7 @@ sub _update_repo { my $project; # 1 - creates new branch if --rename-head if ($self->config->rename_head) { -my $project = $self->api->project($_[0]); +my $project = $self->api->project($_->[0]); if ($project->{default_branch} ne $self->config->dest_branch) { $self->api->create_branch( $id, @@ -57,7 +57,7 @@ sub _update_repo { $configparams->{default_branch} = $self->config->dest_branch; } else { -ds_verbose "Head already renamed for $_[1]"; +ds_verbose "Head already renamed for $_->[1]"; $project = undef; } }
Bug#890594: salsa script ready to review
On Sun, 25 Nov 2018, Raphael Hertzog wrote: > On Thu, 22 Nov 2018, Xavier wrote: > > Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? > > I wanted to do some test but something else broke: > > $ salsa --conf-file +./salsa-pkg-security.conf update_safe sandsifter > --verbose --debug > salsa info: Project sandsifter => pkg-security-team/sandsifter > salsa info: pkg-security-team/sandsifter id is 30154 > sandsifter: > bad description: x86 processor fuzzer > 1 packages misconfigured, update them ? (Y/n) > salsa info: Project sandsifter => pkg-security-team/sandsifter > salsa info: Configuring sandsifter > salsa warn: The #1 argument ($project_id) to project must be a scalar at > /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 49. Forgot to show my file: $ cat salsa-pkg-security.conf SALSA_GROUP=pkg-security-team #SALSA_GROUP_ID=2586 # Skip the pkg-security-team project (containing this file) SALSA_SKIP=pkg-security-team SALSA_DESC_PATTERN="%p packaging" SALSA_DESC=yes SALSA_EMAIL=yes SALSA_EMAIL_RECIPIENTS=dispa...@tracker.debian.org SALSA_IRKER=yes SALSA_IRKER_HOST=ruprecht.snow-crash.org SALSA_IRC_CHANNEL=debian-pkg-security SALSA_TAGPENDING=yes SALSA_SOURCE_BRANCH=master SALSA_DEST_BRANCH=debian/master SALSA_RENAME_HEAD=yes SALSA_ENABLE_MR=yes SALSA_DISABLE_ISSUES=yes Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
On Thu, 22 Nov 2018, Xavier wrote: > Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? I wanted to do some test but something else broke: $ salsa --conf-file +./salsa-pkg-security.conf update_safe sandsifter --verbose --debug salsa info: Project sandsifter => pkg-security-team/sandsifter salsa info: pkg-security-team/sandsifter id is 30154 sandsifter: bad description: x86 processor fuzzer 1 packages misconfigured, update them ? (Y/n) salsa info: Project sandsifter => pkg-security-team/sandsifter salsa info: Configuring sandsifter salsa warn: The #1 argument ($project_id) to project must be a scalar at /usr/share/perl5/Devscripts/Salsa/update_repo.pm line 49. On Fri, 23 Nov 2018, Xavier wrote: > One important thing: I set ruprecht.snow-crash.org as default value for > --irker-host. Is it a good idea ? Is this server secured [1] ? > > [1]: http://www.catb.org/~esr/irker/security.html I don't know. But this server is maintained by a salsa admin so I guess it's fine and he probably restricted it so that only salsa can talk to it (if not he can likely easily do it). Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 22/11/2018 à 22:45, Xavier a écrit : > Le 22/11/2018 à 21:06, Xavier a écrit : >> Le 22/11/2018 à 10:19, Xavier a écrit : >>> Le 22/11/2018 à 08:46, Raphael Hertzog a écrit : Hi, On Wed, 21 Nov 2018, Xavier wrote: > Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM > has been replaced by SALSA_GROUP (same for all team commands/options > replaced by *group*). Working much better with SALSA_GROUP ;) But then I got this: wapiti: bad irc channel: #debian-pkg-security It was really not clear what was wrong. But it seems that you expect: SALSA_IRC_CHANNEL=debian-pkg-security And not the value that I had put initially: SALSA_IRC_CHANNEL=#debian-pkg-security It seems strange to require to strip the leading hash. I would rather be more user-friendly: add the leading hash if it's missing, but otherwise assume that the value is the full name (some channels can start with two leading hashes). Also the documentation should be clear on this. >> >> I updated doc for this (also explanation that "#" is considered as >> comment by "sh"). Spelling errors also fixed, thanks ! >> >>> Hello, >>> >>> this is due to sh. This diff explains more: >>> diff --git a/scripts/salsa.pl b/scripts/salsa.pl >>> index 52a174bb..d1751ecf 100755 >>> --- a/scripts/salsa.pl >>> +++ b/scripts/salsa.pl >>> @@ -567,9 +567,19 @@ C<.devscripts> values: B >>> (yes/ignore/no, default: ignore) >>> >>> =item B<--irc-channel> >>> >>> -IRC channel for KGB or Irker. >>> +IRC channel for KGB or Irker. Can me used more than one time only with >>> +B<--irker>. >>> >>> -C<.devscript> value: B >>> +B: channel must not include the first "#". If salsa find a >>> channel >>> +starting with "#", it will consider that channel starts with 2 "#"! >>> + >>> +C<.devscript> value: B. >>> + >>> +Multiple values must be space separated. >>> + >>> +Since configuration files are read using B, be careful when using >>> "#": you >>> +must enclode the channel with quotes, else B will consider it as a >>> comment >>> +and will ignore this value. >>> >>> =item B<--irker>, B<--no-irker>, B<--disable-irker> >>> Another detail I noticed, the values of SALSA_EMAIL_RECIPIENTS should benefit from the same substitution as SALSA_DESC_PATTERN so that we can include the name of the repo in the generated email addresses. >>> >>> OK, I'm going to do this >> >> Done. Hope salsa works fine now ;-) >> .deb updated >> >> Cheers, >> Xavier > > Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? > > Cheers, > Xavier One important thing: I set ruprecht.snow-crash.org as default value for --irker-host. Is it a good idea ? Is this server secured [1] ? [1]: http://www.catb.org/~esr/irker/security.html
Bug#890594: salsa script ready to review
Le 22/11/2018 à 21:06, Xavier a écrit : > Le 22/11/2018 à 10:19, Xavier a écrit : >> Le 22/11/2018 à 08:46, Raphael Hertzog a écrit : >>> Hi, >>> >>> On Wed, 21 Nov 2018, Xavier wrote: Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM has been replaced by SALSA_GROUP (same for all team commands/options replaced by *group*). >>> >>> Working much better with SALSA_GROUP ;) But then I got this: >>> >>> wapiti: >>> bad irc channel: #debian-pkg-security >>> >>> It was really not clear what was wrong. But it seems that you expect: >>> SALSA_IRC_CHANNEL=debian-pkg-security >>> >>> And not the value that I had put initially: >>> SALSA_IRC_CHANNEL=#debian-pkg-security >>> >>> It seems strange to require to strip the leading hash. I would rather >>> be more user-friendly: add the leading hash if it's missing, but otherwise >>> assume that the value is the full name (some channels can start with two >>> leading hashes). Also the documentation should be clear on this. > > I updated doc for this (also explanation that "#" is considered as > comment by "sh"). Spelling errors also fixed, thanks ! > >> Hello, >> >> this is due to sh. This diff explains more: >> diff --git a/scripts/salsa.pl b/scripts/salsa.pl >> index 52a174bb..d1751ecf 100755 >> --- a/scripts/salsa.pl >> +++ b/scripts/salsa.pl >> @@ -567,9 +567,19 @@ C<.devscripts> values: B >> (yes/ignore/no, default: ignore) >> >> =item B<--irc-channel> >> >> -IRC channel for KGB or Irker. >> +IRC channel for KGB or Irker. Can me used more than one time only with >> +B<--irker>. >> >> -C<.devscript> value: B >> +B: channel must not include the first "#". If salsa find a >> channel >> +starting with "#", it will consider that channel starts with 2 "#"! >> + >> +C<.devscript> value: B. >> + >> +Multiple values must be space separated. >> + >> +Since configuration files are read using B, be careful when using >> "#": you >> +must enclode the channel with quotes, else B will consider it as a >> comment >> +and will ignore this value. >> >> =item B<--irker>, B<--no-irker>, B<--disable-irker> >> >>> Another detail I noticed, the values of SALSA_EMAIL_RECIPIENTS should >>> benefit >>> from the same substitution as SALSA_DESC_PATTERN so that we can include the >>> name of the repo in the generated email addresses. >> >> OK, I'm going to do this > > Done. Hope salsa works fine now ;-) > .deb updated > > Cheers, > Xavier Last .deb contains the SALSA_RENAME_HEAD. Could you test it ? Cheers, Xavier
Bug#890594: salsa script ready to review
Le 22/11/2018 à 10:19, Xavier a écrit : > Le 22/11/2018 à 08:46, Raphael Hertzog a écrit : >> Hi, >> >> On Wed, 21 Nov 2018, Xavier wrote: >>> Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM >>> has been replaced by SALSA_GROUP (same for all team commands/options >>> replaced by *group*). >> >> Working much better with SALSA_GROUP ;) But then I got this: >> >> wapiti: >> bad irc channel: #debian-pkg-security >> >> It was really not clear what was wrong. But it seems that you expect: >> SALSA_IRC_CHANNEL=debian-pkg-security >> >> And not the value that I had put initially: >> SALSA_IRC_CHANNEL=#debian-pkg-security >> >> It seems strange to require to strip the leading hash. I would rather >> be more user-friendly: add the leading hash if it's missing, but otherwise >> assume that the value is the full name (some channels can start with two >> leading hashes). Also the documentation should be clear on this. I updated doc for this (also explanation that "#" is considered as comment by "sh"). Spelling errors also fixed, thanks ! > Hello, > > this is due to sh. This diff explains more: > diff --git a/scripts/salsa.pl b/scripts/salsa.pl > index 52a174bb..d1751ecf 100755 > --- a/scripts/salsa.pl > +++ b/scripts/salsa.pl > @@ -567,9 +567,19 @@ C<.devscripts> values: B > (yes/ignore/no, default: ignore) > > =item B<--irc-channel> > > -IRC channel for KGB or Irker. > +IRC channel for KGB or Irker. Can me used more than one time only with > +B<--irker>. > > -C<.devscript> value: B > +B: channel must not include the first "#". If salsa find a > channel > +starting with "#", it will consider that channel starts with 2 "#"! > + > +C<.devscript> value: B. > + > +Multiple values must be space separated. > + > +Since configuration files are read using B, be careful when using > "#": you > +must enclode the channel with quotes, else B will consider it as a > comment > +and will ignore this value. > > =item B<--irker>, B<--no-irker>, B<--disable-irker> > >> Another detail I noticed, the values of SALSA_EMAIL_RECIPIENTS should benefit >> from the same substitution as SALSA_DESC_PATTERN so that we can include the >> name of the repo in the generated email addresses. > > OK, I'm going to do this Done. Hope salsa works fine now ;-) .deb updated Cheers, Xavier
Bug#890594: salsa script ready to review
On Thu, 22 Nov 2018, Xavier wrote: > -IRC channel for KGB or Irker. > +IRC channel for KGB or Irker. Can me used more than one time only with > +B<--irker>. s/me/be/ > -C<.devscript> value: B > +B: channel must not include the first "#". If salsa find a channel s/find/finds/ > +starting with "#", it will consider that channel starts with 2 "#"! s/that channel/that the channel/ > +Since configuration files are read using B, be careful when using "#": > you > +must enclode the channel with quotes, else B will consider it as a > comment s/enclode/enclose/ Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
On Thu, 22 Nov 2018, Raphael Hertzog wrote: > wapiti: > bad irc channel: #debian-pkg-security Even after changing the setting, I still had the message for a few packages where the exact value was "#debian-pkg-security " (trailing space). I don't know if you want to strip the spaces before comparison but at the very least you might want to add some quotes to the error message so that it's clear that the difference comes from a trailing space. Since everything looked fine, I ran "update_repo" but was disappointed to see that it did change things even on packages that were fine: $ salsa --conf-file +./pkg-security.conf check_repo --verbose --all salsa info: Found 220 projects [...] salsa info: sandsifter: OK [...] $ salsa --conf-file +./pkg-security.conf update_repo --verbose --all You're going to configure 220 projects, continue (N/y)?y [...] salsa info: Configuring sandsifter salsa warn: Deleting old irker (redirected to #debian-pkg-security) salsa info: Irker hook added to project 30154 (channel: debian-pkg-security) salsa warn: Deleting old email-on-push (redirected to dispa...@tracker.debian.org) salsa info: Email-on-push hook added to project 30154 (recipients: dispa...@tracker.debian.org) salsa warn: Deleting old tagpending (was https://webhook.salsa.debian.org/tagpending/sandsifter) salsa info: Tagpending hook added to project 30154 salsa info: Project 30154 updated Worried by this, I pressed CTRL+C in the middle and the script was interrupted after a "Deleting old tagpending" (but before the corresponding "Tagpending hook added"), maybe we could catch the kill request and stop gracefully between two packages ? Then in the list of packages, I had "forensic-artifacts" which was using "master" and not "debian/master" and it didn't detect the problem and did not solve it either. I see the manual page doesn't mention any variable name for the --rename-head option. I tried to guess the name and I have put this in the configuration file: SALSA_SOURCE_BRANCH=master SALSA_DEST_BRANCH=debian/master SALSA_RENAME_HEAD=yes But it doesn't seem to do the trick. Is there a reason for this behaviour? However when I run check_repo with the command line option, then it sees the problem: $ salsa --conf-file +./pkg-security.conf check_repo --verbose --rename-head pkg-security-team/forensic-artifacts salsa info: pkg-security-team/forensic-artifacts id is 29265 pkg-security-team/forensic-artifacts: Default branch is master Then I ran the update_repo command and it didn't say anything about the fact that it was fixing the default branch: $ salsa --conf-file +./pkg-security.conf update_repo --verbose --rename-head pkg-security-team/forensic-artifacts salsa info: pkg-security-team/forensic-artifacts id is 29265 salsa info: Configuring pkg-security-team/forensic-artifacts salsa warn: Deleting old irker (redirected to #debian-pkg-security) salsa info: Irker hook added to project 29265 (channel: debian-pkg-security) salsa warn: Deleting old email-on-push (redirected to dispa...@tracker.debian.org) salsa info: Email-on-push hook added to project 29265 (recipients: dispa...@tracker.debian.org) salsa warn: Deleting old tagpending (was https://webhook.salsa.debian.org/tagpending/forensic-artifacts) salsa info: Tagpending hook added to project 29265 salsa info: Project 29265 updated But the change was well done (I verified it on the web interface). Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 22/11/2018 à 08:46, Raphael Hertzog a écrit : > Hi, > > On Wed, 21 Nov 2018, Xavier wrote: >> Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM >> has been replaced by SALSA_GROUP (same for all team commands/options >> replaced by *group*). > > Working much better with SALSA_GROUP ;) But then I got this: > > wapiti: > bad irc channel: #debian-pkg-security > > It was really not clear what was wrong. But it seems that you expect: > SALSA_IRC_CHANNEL=debian-pkg-security > > And not the value that I had put initially: > SALSA_IRC_CHANNEL=#debian-pkg-security > > It seems strange to require to strip the leading hash. I would rather > be more user-friendly: add the leading hash if it's missing, but otherwise > assume that the value is the full name (some channels can start with two > leading hashes). Also the documentation should be clear on this. Hello, this is due to sh. This diff explains more: diff --git a/scripts/salsa.pl b/scripts/salsa.pl index 52a174bb..d1751ecf 100755 --- a/scripts/salsa.pl +++ b/scripts/salsa.pl @@ -567,9 +567,19 @@ C<.devscripts> values: B (yes/ignore/no, default: ignore) =item B<--irc-channel> -IRC channel for KGB or Irker. +IRC channel for KGB or Irker. Can me used more than one time only with +B<--irker>. -C<.devscript> value: B +B: channel must not include the first "#". If salsa find a channel +starting with "#", it will consider that channel starts with 2 "#"! + +C<.devscript> value: B. + +Multiple values must be space separated. + +Since configuration files are read using B, be careful when using "#": you +must enclode the channel with quotes, else B will consider it as a comment +and will ignore this value. =item B<--irker>, B<--no-irker>, B<--disable-irker> > Another detail I noticed, the values of SALSA_EMAIL_RECIPIENTS should benefit > from the same substitution as SALSA_DESC_PATTERN so that we can include the > name of the repo in the generated email addresses. OK, I'm going to do this > Cheers, >
Bug#890594: salsa script ready to review
Hi, On Wed, 21 Nov 2018, Xavier wrote: > Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM > has been replaced by SALSA_GROUP (same for all team commands/options > replaced by *group*). Working much better with SALSA_GROUP ;) But then I got this: wapiti: bad irc channel: #debian-pkg-security It was really not clear what was wrong. But it seems that you expect: SALSA_IRC_CHANNEL=debian-pkg-security And not the value that I had put initially: SALSA_IRC_CHANNEL=#debian-pkg-security It seems strange to require to strip the leading hash. I would rather be more user-friendly: add the leading hash if it's missing, but otherwise assume that the value is the full name (some channels can start with two leading hashes). Also the documentation should be clear on this. Another detail I noticed, the values of SALSA_EMAIL_RECIPIENTS should benefit from the same substitution as SALSA_DESC_PATTERN so that we can include the name of the repo in the generated email addresses. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 21/11/2018 à 23:04, Xavier a écrit : > > Le 21/11/2018 à 23:00, Xavier a écrit : >> Le 21/11/2018 à 22:57, Xavier a écrit : >>> Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : On Tue, 20 Nov 2018, Xavier wrote: >> Unable to reproduce now, this bug seems fixed now. I'm going to provide >> a new .deb after adding other requests. > > I built a new .deb: > http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb It's actually worse. I'm attaching my pkg-security.conf and here's the call I'm doing: $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all salsa info: Found 7 projects grub: email-on-push missing irker missing tagpending missing debian.org: email-on-push missing irker missing tagpending missing ^C Those packages are not part of pkg-security... My .devscripts has this: $ grep SALSA ~/.devscripts SALSA_TOKEN_FILE=/private/config-sensible/salsa-token $ cat /private/config-sensible/salsa-token SALSA_TOKEN=xx SALSA_URL=https://salsa.debian.org/api/v4 >>> >>> SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts >>> if you want to overwrite default value (not needed) >>> >>> I don't see any SALSA_GROUP in your conf file, so salsa looks at your >>> forks. The "grub" project seen above is "hertzog/grub" which exists ;-) >> >> Seems to work fine with "SALSA_GROUP security-tracker-team" (not really >> tested since I've no rights on this group but tested on another group) > > SALSA_GROUP=security-tracker-team > > or > > SALSA_GROUP_ID=2102 Sorry, I found a SALSA_TEAM in your conf file. For clarity, SALSA_TEAM has been replaced by SALSA_GROUP (same for all team commands/options replaced by *group*).
Bug#890594: salsa script ready to review
Le 21/11/2018 à 23:08, Xavier a écrit : > Le 21/11/2018 à 23:04, Xavier a écrit : >> >> Le 21/11/2018 à 23:00, Xavier a écrit : >>> Le 21/11/2018 à 22:57, Xavier a écrit : Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : > On Tue, 20 Nov 2018, Xavier wrote: >>> Unable to reproduce now, this bug seems fixed now. I'm going to provide >>> a new .deb after adding other requests. >> >> I built a new .deb: >> http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb > > It's actually worse. I'm attaching my pkg-security.conf and here's the > call I'm doing: > > $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose > --all > salsa info: Found 7 projects > grub: > email-on-push missing > irker missing > tagpending missing > debian.org: > email-on-push missing > irker missing > tagpending missing > ^C > > Those packages are not part of pkg-security... > > My .devscripts has this: > $ grep SALSA ~/.devscripts > SALSA_TOKEN_FILE=/private/config-sensible/salsa-token > $ cat /private/config-sensible/salsa-token > SALSA_TOKEN=xx > SALSA_URL=https://salsa.debian.org/api/v4 SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts if you want to overwrite default value (not needed) > > salsa looks only for a token in /private/config-sensible/salsa-token. > Any other values are ignored > I don't see any SALSA_GROUP in your conf file, so salsa looks at your forks. The "grub" project seen above is "hertzog/grub" which exists ;-) >>> >>> Seems to work fine with "SALSA_GROUP security-tracker-team" (not really >>> tested since I've no rights on this group but tested on another group) >> >> SALSA_GROUP=security-tracker-team >> >> or >> >> SALSA_GROUP_ID=2102 Another successful test using https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/blob/master/.salsa.conf on OW2 GitLab $ salsa --conf-file .salsa.conf check_repo --all --verbose salsa.pl info: Found 1 projects salsa.pl info: lemonldap-ng: OK $ cat .salsa.conf SALSA_TOKEN=`cat ~/.ow2-token` SALSA_API_URL=https://gitlab.ow2.org/api/v4 SALSA_GIT_SERVER_URL=g...@gitlab.ow2.org: SALSA_GROUP_ID=34 SALSA_IRKER=yes SALSA_IRKER_HOST=irker.ow2.org SALSA_IRKER_PORT=6659 SALSA_IRKER_SERVER_URL=ircs://chat.freenode.net SALSA_IRC_CHANNEL=lemonldap-ng SALSA_EMAIL=yes SALSA_EMAIL_RECIPIENTS=lemonldap-ng-chan...@ow2.org
Bug#890594: salsa script ready to review
Le 21/11/2018 à 23:04, Xavier a écrit : > > Le 21/11/2018 à 23:00, Xavier a écrit : >> Le 21/11/2018 à 22:57, Xavier a écrit : >>> Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : On Tue, 20 Nov 2018, Xavier wrote: >> Unable to reproduce now, this bug seems fixed now. I'm going to provide >> a new .deb after adding other requests. > > I built a new .deb: > http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb It's actually worse. I'm attaching my pkg-security.conf and here's the call I'm doing: $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all salsa info: Found 7 projects grub: email-on-push missing irker missing tagpending missing debian.org: email-on-push missing irker missing tagpending missing ^C Those packages are not part of pkg-security... My .devscripts has this: $ grep SALSA ~/.devscripts SALSA_TOKEN_FILE=/private/config-sensible/salsa-token $ cat /private/config-sensible/salsa-token SALSA_TOKEN=xx SALSA_URL=https://salsa.debian.org/api/v4 >>> >>> SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts >>> if you want to overwrite default value (not needed) salsa looks only for a token in /private/config-sensible/salsa-token. Any other values are ignored >>> I don't see any SALSA_GROUP in your conf file, so salsa looks at your >>> forks. The "grub" project seen above is "hertzog/grub" which exists ;-) >> >> Seems to work fine with "SALSA_GROUP security-tracker-team" (not really >> tested since I've no rights on this group but tested on another group) > > SALSA_GROUP=security-tracker-team > > or > > SALSA_GROUP_ID=2102 > (I just replaced the token, the rest is exactly as-is) Cheers, >>
Bug#890594: salsa script ready to review
Le 21/11/2018 à 23:00, Xavier a écrit : > Le 21/11/2018 à 22:57, Xavier a écrit : >> Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : >>> On Tue, 20 Nov 2018, Xavier wrote: > Unable to reproduce now, this bug seems fixed now. I'm going to provide > a new .deb after adding other requests. I built a new .deb: http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb >>> >>> It's actually worse. I'm attaching my pkg-security.conf and here's the >>> call I'm doing: >>> >>> $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all >>> salsa info: Found 7 projects >>> grub: >>> email-on-push missing >>> irker missing >>> tagpending missing >>> debian.org: >>> email-on-push missing >>> irker missing >>> tagpending missing >>> ^C >>> >>> Those packages are not part of pkg-security... >>> >>> My .devscripts has this: >>> $ grep SALSA ~/.devscripts >>> SALSA_TOKEN_FILE=/private/config-sensible/salsa-token >>> $ cat /private/config-sensible/salsa-token >>> SALSA_TOKEN=xx >>> SALSA_URL=https://salsa.debian.org/api/v4 >> >> SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts >> if you want to overwrite default value (not needed) >> >> I don't see any SALSA_GROUP in your conf file, so salsa looks at your >> forks. The "grub" project seen above is "hertzog/grub" which exists ;-) > > Seems to work fine with "SALSA_GROUP security-tracker-team" (not really > tested since I've no rights on this group but tested on another group) SALSA_GROUP=security-tracker-team or SALSA_GROUP_ID=2102 >>> (I just replaced the token, the rest is exactly as-is) >>> >>> Cheers, >
Bug#890594: salsa script ready to review
Le 21/11/2018 à 22:57, Xavier a écrit : > Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : >> On Tue, 20 Nov 2018, Xavier wrote: Unable to reproduce now, this bug seems fixed now. I'm going to provide a new .deb after adding other requests. >>> >>> I built a new .deb: >>> http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb >> >> It's actually worse. I'm attaching my pkg-security.conf and here's the >> call I'm doing: >> >> $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all >> salsa info: Found 7 projects >> grub: >> email-on-push missing >> irker missing >> tagpending missing >> debian.org: >> email-on-push missing >> irker missing >> tagpending missing >> ^C >> >> Those packages are not part of pkg-security... >> >> My .devscripts has this: >> $ grep SALSA ~/.devscripts >> SALSA_TOKEN_FILE=/private/config-sensible/salsa-token >> $ cat /private/config-sensible/salsa-token >> SALSA_TOKEN=xx >> SALSA_URL=https://salsa.debian.org/api/v4 > > SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts > if you want to overwrite default value (not needed) > > I don't see any SALSA_GROUP in your conf file, so salsa looks at your > forks. The "grub" project seen above is "hertzog/grub" which exists ;-) Seems to work fine with "SALSA_GROUP security-tracker-team" (not really tested since I've no rights on this group but tested on another group) >> (I just replaced the token, the rest is exactly as-is) >> >> Cheers,
Bug#890594: salsa script ready to review
Le 21/11/2018 à 22:51, Raphael Hertzog a écrit : > On Tue, 20 Nov 2018, Xavier wrote: >>> Unable to reproduce now, this bug seems fixed now. I'm going to provide >>> a new .deb after adding other requests. >> >> I built a new .deb: >> http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb > > It's actually worse. I'm attaching my pkg-security.conf and here's the > call I'm doing: > > $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all > salsa info: Found 7 projects > grub: > email-on-push missing > irker missing > tagpending missing > debian.org: > email-on-push missing > irker missing > tagpending missing > ^C > > Those packages are not part of pkg-security... > > My .devscripts has this: > $ grep SALSA ~/.devscripts > SALSA_TOKEN_FILE=/private/config-sensible/salsa-token > $ cat /private/config-sensible/salsa-token > SALSA_TOKEN=xx > SALSA_URL=https://salsa.debian.org/api/v4 SALSA_URL here isn't read. Set it in ./pkg-security.conf or .devscripts if you want to overwrite default value (not needed) I don't see any SALSA_GROUP in your conf file, so salsa looks at your forks. The "grub" project seen above is "hertzog/grub" which exists ;-) > (I just replaced the token, the rest is exactly as-is) > > Cheers, >
Bug#890594: salsa script ready to review
On Tue, 20 Nov 2018, Xavier wrote: > > Unable to reproduce now, this bug seems fixed now. I'm going to provide > > a new .deb after adding other requests. > > I built a new .deb: > http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb It's actually worse. I'm attaching my pkg-security.conf and here's the call I'm doing: $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all salsa info: Found 7 projects grub: email-on-push missing irker missing tagpending missing debian.org: email-on-push missing irker missing tagpending missing ^C Those packages are not part of pkg-security... My .devscripts has this: $ grep SALSA ~/.devscripts SALSA_TOKEN_FILE=/private/config-sensible/salsa-token $ cat /private/config-sensible/salsa-token SALSA_TOKEN=xx SALSA_URL=https://salsa.debian.org/api/v4 (I just replaced the token, the rest is exactly as-is) Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/ SALSA_TEAM=pkg-security-team #SALSA_TEAM_ID=2586 SALSA_IRC_CHANNEL=#debian-pkg-security #SALSA_DESC=yes SALSA_EMAIL=yes SALSA_EMAIL_RECIPIENTS=dispa...@tracker.debian.org SALSA_IRKER=yes SALSA_IRKER_HOST=ruprecht.snow-crash.org SALSA_TAGPENDING=yes SALSA_SOURCE_BRANCH=master SALSA_DEST_BRANCH=debian/master SALSA_RENAME_HEAD=yes SALSA_ENABLE_MR=yes SALSA_DISABLE_ISSUES=yes
Bug#890594: salsa script ready to review
Le 20/11/2018 à 16:36, Xavier a écrit : > Le 20/11/2018 à 15:53, Xavier a écrit : >> Le 20/11/2018 à 15:10, Raphael Hertzog a écrit : >>> Hi Xavier, >> >> Hi Raphaël, thanks for your report! >> >>> On Wed, 07 Nov 2018, Xavier wrote: I think teams should publish their own salsa.conf files. If you agree, I propose to stop adding feature to let Martin review the actual code and add it to devscripts. Then after some tests in real life, we will be able to continue adding new commands or options. >>> >>> I tried to use the tool for pkg-security today and I stumbled on a few >>> issues: >>> - no way to enable the "Emails on push" integration which is the most >>> important thing to configure for me >> >> OK, I'm going to add this done >>> - no support for the "Irker (IRC gateway)" integration either (we >>> currently prefer the less verbose notifications of irker) >> >> OK, let's add it done >>> See "Creating new repositories" on >>> https://wiki.debian.org/Teams/pkg-security >>> >>> I also had SALSA_TOKEN="x" in a file referenced by SALSA_TOKEN_FILE >>> in my main configuration file and this did not work, I had to stip the >>> double quotes. >> >> Right, I'm going to fix this. >> Since devscripts loads its config file using sh, you can reference your >> file using: >> >> # ~/.devscripts.conf >> source ~/my-token >> >> # ~/my-token >> SALSA_TOKEN="x" >> >> This is not optimal but works also with double quotes done >>> The manual page does not document the value that we have to put into >>> boolean variables (such as SALSA_TAGPENDING). I assume it's "yes/no" >>> (or that yes/no should work). I have put this in my config file: >>> SALSA_ENABLE_MR=yes >>> >>> Yet the tool always tells me that the variable merge_requests_enabled >>> should be set to zero: >>> $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all >>> acct: >>> merge_requests_enabled should be 0 >>> [...] >> >> This is a bug. looking... > > Unable to reproduce now, this bug seems fixed now. I'm going to provide > a new .deb after adding other requests. I built a new .deb: http://people.debian.org/~yadd/devscripts_2.18.10_amd64.deb >>> I used the last .deb that you built even if it's now older than the >>> current version in sid. >>> >>> Cheers, >> >> salsa isn't yet in sid, only in my fork >> >> Cheers, >> Xavier
Bug#890594: salsa script ready to review
Le 20/11/2018 à 15:53, Xavier a écrit : > Le 20/11/2018 à 15:10, Raphael Hertzog a écrit : >> Hi Xavier, > > Hi Raphaël, thanks for your report! > >> On Wed, 07 Nov 2018, Xavier wrote: >>> I think teams should publish their own salsa.conf files. If you agree, I >>> propose to stop adding feature to let Martin review the actual code and >>> add it to devscripts. Then after some tests in real life, we will be >>> able to continue adding new commands or options. >> >> I tried to use the tool for pkg-security today and I stumbled on a few >> issues: >> - no way to enable the "Emails on push" integration which is the most >> important thing to configure for me > > OK, I'm going to add this > >> - no support for the "Irker (IRC gateway)" integration either (we >> currently prefer the less verbose notifications of irker) > > OK, let's add it > >> See "Creating new repositories" on https://wiki.debian.org/Teams/pkg-security >> >> I also had SALSA_TOKEN="x" in a file referenced by SALSA_TOKEN_FILE >> in my main configuration file and this did not work, I had to stip the >> double quotes. > > Right, I'm going to fix this. > Since devscripts loads its config file using sh, you can reference your > file using: > > # ~/.devscripts.conf > source ~/my-token > > # ~/my-token > SALSA_TOKEN="x" > > This is not optimal but works also with double quotes > >> The manual page does not document the value that we have to put into >> boolean variables (such as SALSA_TAGPENDING). I assume it's "yes/no" >> (or that yes/no should work). I have put this in my config file: >> SALSA_ENABLE_MR=yes >> >> Yet the tool always tells me that the variable merge_requests_enabled >> should be set to zero: >> $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all >> acct: >> merge_requests_enabled should be 0 >> [...] > > This is a bug. looking... Unable to reproduce now, this bug seems fixed now. I'm going to provide a new .deb after adding other requests. >> I used the last .deb that you built even if it's now older than the >> current version in sid. >> >> Cheers, > > salsa isn't yet in sid, only in my fork > > Cheers, > Xavier >
Bug#890594: salsa script ready to review
Le 20/11/2018 à 15:10, Raphael Hertzog a écrit : > Hi Xavier, Hi Raphaël, thanks for your report! > On Wed, 07 Nov 2018, Xavier wrote: >> I think teams should publish their own salsa.conf files. If you agree, I >> propose to stop adding feature to let Martin review the actual code and >> add it to devscripts. Then after some tests in real life, we will be >> able to continue adding new commands or options. > > I tried to use the tool for pkg-security today and I stumbled on a few > issues: > - no way to enable the "Emails on push" integration which is the most > important thing to configure for me OK, I'm going to add this > - no support for the "Irker (IRC gateway)" integration either (we > currently prefer the less verbose notifications of irker) OK, let's add it > See "Creating new repositories" on https://wiki.debian.org/Teams/pkg-security > > I also had SALSA_TOKEN="x" in a file referenced by SALSA_TOKEN_FILE > in my main configuration file and this did not work, I had to stip the > double quotes. Right, I'm going to fix this. Since devscripts loads its config file using sh, you can reference your file using: # ~/.devscripts.conf source ~/my-token # ~/my-token SALSA_TOKEN="x" This is not optimal but works also with double quotes > The manual page does not document the value that we have to put into > boolean variables (such as SALSA_TAGPENDING). I assume it's "yes/no" > (or that yes/no should work). I have put this in my config file: > SALSA_ENABLE_MR=yes > > Yet the tool always tells me that the variable merge_requests_enabled > should be set to zero: > $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all > acct: > merge_requests_enabled should be 0 > [...] This is a bug. looking... > I used the last .deb that you built even if it's now older than the > current version in sid. > > Cheers, salsa isn't yet in sid, only in my fork Cheers, Xavier
Bug#890594: salsa script ready to review
Hi Xavier, On Wed, 07 Nov 2018, Xavier wrote: > I think teams should publish their own salsa.conf filesIf you agree, I > propose to stop adding feature to let Martin review the actual code and > add it to devscripts. Then after some tests in real life, we will be > able to continue adding new commands or options. I tried to use the tool for pkg-security today and I stumbled on a few issues: - no way to enable the "Emails on push" integration which is the most important thing to configure for me - no support for the "Irker (IRC gateway)" integration either (we currently prefer the less verbose notifications of irker) See "Creating new repositories" on https://wiki.debian.org/Teams/pkg-security I also had SALSA_TOKEN="x" in a file referenced by SALSA_TOKEN_FILE in my main configuration file and this did not work, I had to stip the double quotes. The manual page does not document the value that we have to put into boolean variables (such as SALSA_TAGPENDING). I assume it's "yes/no" (or that yes/no should work). I have put this in my config file: SALSA_ENABLE_MR=yes Yet the tool always tells me that the variable merge_requests_enabled should be set to zero: $ salsa --conf-file +./pkg-security.conf check_repo --debug --verbose --all acct: merge_requests_enabled should be 0 [...] I used the last .deb that you built even if it's now older than the current version in sid. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le Mercredi, Novembre 07, 2018 00:04 CET, Xavier a écrit:Le 06/11/2018 à 14:04, Raphael Hertzog a écrit :> On Tue, 06 Nov 2018, Xavier wrote: >>> While reading the discussion on -devel, it also occured to me that we >>> might want to be able to enable/disable the possibility to file issues >>> and open merge request. >> >> MR is done. Example: > > You misunderstood my request. I'm speaking of setting the "issues_enabled" > and "merge_requests_enabled" attribute of the projects: > https://docs.gitlab.com/ce/api/projects.html#edit-project OK, done in last commit. New package available at http://people.debian.org/~yadd/devscripts_2.18.8_amd64.deb >>> - you override/revert the default settings for some special repositories >>> (typically those that are not packaging related, i.e. team scripts, >>> team website, git repo with submodules of all packages, etc.). >> >> With a list given in a file ? Something like that ? >> SALSA_TEAM_SKIP_FILE=/usr/share/pkg-js-tools/skip-projects > > Ok, that looks good enough, thanks. My idea was to have everything > in one file but it's fine to just be able to exclude projects from > the processing. Little complex to do... Since excluded packages may not have to change many, you can configure them, then exclude them in your conf file. I think teams should publish their own salsa.conf filesIf you agree, I propose to stop adding feature to let Martin review the actual code and add it to devscripts. Then after some tests in real life, we will be able to continue adding new commands or options. Cheers, Xavier
Bug#890594: salsa script ready to review
Le 06/11/2018 à 14:04, Raphael Hertzog a écrit : > On Tue, 06 Nov 2018, Xavier wrote: >>> While reading the discussion on -devel, it also occured to me that we >>> might want to be able to enable/disable the possibility to file issues >>> and open merge request. >> >> MR is done. Example: > > You misunderstood my request. I'm speaking of setting the "issues_enabled" > and "merge_requests_enabled" attribute of the projects: > https://docs.gitlab.com/ce/api/projects.html#edit-project OK, done in last commit. New package available at http://people.debian.org/~yadd/devscripts_2.18.8_amd64.deb >>> - you override/revert the default settings for some special repositories >>> (typically those that are not packaging related, i.e. team scripts, >>> team website, git repo with submodules of all packages, etc.). >> >> With a list given in a file ? Something like that ? >> SALSA_TEAM_SKIP_FILE=/usr/share/pkg-js-tools/skip-projects > > Ok, that looks good enough, thanks. My idea was to have everything > in one file but it's fine to just be able to exclude projects from > the processing. Little complex to do... Since excluded packages may not have to change many, you can configure them, then exclude them in your conf file. I think teams should publish their own salsa.conf files
Bug#890594: salsa script ready to review
On Tue, 06 Nov 2018, Xavier wrote: > > While reading the discussion on -devel, it also occured to me that we > > might want to be able to enable/disable the possibility to file issues > > and open merge request. > > MR is done. Example: You misunderstood my request. I'm speaking of setting the "issues_enabled" and "merge_requests_enabled" attribute of the projects: https://docs.gitlab.com/ce/api/projects.html#edit-project > > - you override/revert the default settings for some special repositories > > (typically those that are not packaging related, i.e. team scripts, > > team website, git repo with submodules of all packages, etc.). > > With a list given in a file ? Something like that ? > SALSA_TEAM_SKIP_FILE=/usr/share/pkg-js-tools/skip-projects Ok, that looks good enough, thanks. My idea was to have everything in one file but it's fine to just be able to exclude projects from the processing. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 06/11/2018 à 12:11, Raphael Hertzog a écrit : > On Mon, 29 Oct 2018, Xavier wrote: >> I also added an "update_safe" command that starts "check_repo", then >> reports and asks before launching "update_repo" (unless --yes). I think >> that's what you want, isn't it? > > Yes, that's the idea. > > While reading the discussion on -devel, it also occured to me that we > might want to be able to enable/disable the possibility to file issues > and open merge request. MR is done. Example: $ salsa fork debian/devscripts $ cd devscripts $ # do change $ git commit -a -m "uscan: fix something" -m "Closes: #111" $ salsa mr For issues, it may be confuse with BTS, isn't it ? Else it is easy to do. > And it would be nice if the configuration file could have some sort > of override logic: > - you define the default setting for all repositories in a group Easy. Example: $ alias js_admin='salsa --conf-file=~/usr/share/pkg-js-tools/salsa.conf' $ cat /usr/share/pkg-js-tools/salsa.conf SALSA_TEAM=js-team SALSA_KGB=yes SALSA_IRC_CHANNEL=debian-js SALSA_DESC=yes SALSA_DESC_PATTERN="JS-Team package %p" SALSA_TAGPENDING=yes First time: $ js_admin update_repo --all Maintenance: $ js_admin update_safe --all > - you override/revert the default settings for some special repositories > (typically those that are not packaging related, i.e. team scripts, > team website, git repo with submodules of all packages, etc.). With a list given in a file ? Something like that ? SALSA_TEAM_SKIP_FILE=/usr/share/pkg-js-tools/skip-projects and/or SALSA_TEAM_SKIP=js-team.pages.debian.net pkg-js-tools Of course be available in command-line: $ js_admin --skip js-team.pages.debian.net --skip pkg-js-tools \ update_repo --all and/or $ js_admin --skip-file=/usr/share/pkg-js-tools/skip-projects \ update_repo --all > Cheers, Regards, Xavier
Bug#890594: salsa script ready to review
Le 06/11/2018 à 12:13, Raphael Hertzog a écrit : > Hi, > > On Mon, 29 Oct 2018, Xavier wrote: >> I also added an "update_safe" command that starts "check_repo", then >> reports and asks before launching "update_repo" (unless --yes). I think >> that's what you want, isn't it? > > Last point, I would like to test your tool. Since the script is not > standalone, can you make a test package available? > > Cheers, Hello, thanks. Here is a build: http://people.debian.org/~yadd/devscripts_2.18.8_amd64.deb
Bug#890594: salsa script ready to review
Hi, On Mon, 29 Oct 2018, Xavier wrote: > I also added an "update_safe" command that starts "check_repo", then > reports and asks before launching "update_repo" (unless --yes). I think > that's what you want, isn't it? Last point, I would like to test your tool. Since the script is not standalone, can you make a test package available? Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
On Mon, 29 Oct 2018, Xavier wrote: > I also added an "update_safe" command that starts "check_repo", then > reports and asks before launching "update_repo" (unless --yes). I think > that's what you want, isn't it? Yes, that's the idea. While reading the discussion on -devel, it also occured to me that we might want to be able to enable/disable the possibility to file issues and open merge request. And it would be nice if the configuration file could have some sort of override logic: - you define the default setting for all repositories in a group - you override/revert the default settings for some special repositories (typically those that are not packaging related, i.e. team scripts, team website, git repo with submodules of all packages, etc.). Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 29/10/2018 à 21:50, Xavier a écrit : > Le 29/10/2018 à 21:35, Xavier a écrit : >> Le 29/10/2018 à 12:27, Raphael Hertzog a écrit : >>> On Sun, 28 Oct 2018, Xavier wrote: Mattia explained me dep14. I found a way to do it: create branch from master, update project to set default_branch to debian/master then remove master. It works as expected. $ salsa update_repo node-mongodb --group js-team --rename-head $ salsa update_repo --all --rename-head --no-fail # all user projects Manpage updated: https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L339 >>> >>> Nice, thanks! >>> >>> Now, this tool is really powerful and one could be wary of breaking >>> things. It would be nice if there was a "--no-act" option that would not >>> change anything but only display what would be done. >>> >>> It could be useful to see what repositories are currently not following >>> the usual rules and double check that we really want to override their >>> current configuration. >> >> Done: now you have a "check_repo" command that reports bad configured >> repositories without modifying anything. >> >> https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L117 > > It can check: > - kgb/irc channel > - tagpending > - description > - default branch > > $? contains the number of failed packages. > > Examples: > > # Some js-team packages: > $ salsa check_repo --team js-team --desc \ >--kgb --irc-channel=debian-js \ >--tagpending \ >--rename-head \ > node-tap node-mongodb > node-tap: > bad description: Repository imported from > https://anonscm.debian.org/git/pkg-javascript/node-tap.git/ > Default branch is master > kgb missing > Tagpending missing > node-mongodb: > Default branch is master > kgb missing > > > # All perl-team packages: > $ salsa check_repo --team-id=2666 --desc \ >--kgb --irc-channel=debian-perl \ >--tagpending \ >--all > > (takes a very long time) I also added an "update_safe" command that starts "check_repo", then reports and asks before launching "update_repo" (unless --yes). I think that's what you want, isn't it?
Bug#890594: salsa script ready to review
Le 29/10/2018 à 21:35, Xavier a écrit : > Le 29/10/2018 à 12:27, Raphael Hertzog a écrit : >> On Sun, 28 Oct 2018, Xavier wrote: >>> Mattia explained me dep14. I found a way to do it: create branch from >>> master, update project to set default_branch to debian/master then >>> remove master. It works as expected. >>> >>> $ salsa update_repo node-mongodb --group js-team --rename-head >>> >>> $ salsa update_repo --all --rename-head --no-fail # all user projects >>> >>> Manpage updated: >>> https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L339 >> >> Nice, thanks! >> >> Now, this tool is really powerful and one could be wary of breaking >> things. It would be nice if there was a "--no-act" option that would not >> change anything but only display what would be done. >> >> It could be useful to see what repositories are currently not following >> the usual rules and double check that we really want to override their >> current configuration. > > Done: now you have a "check_repo" command that reports bad configured > repositories without modifying anything. > > https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L117 It can check: - kgb/irc channel - tagpending - description - default branch $? contains the number of failed packages. Examples: # Some js-team packages: $ salsa check_repo --team js-team --desc \ --kgb --irc-channel=debian-js \ --tagpending \ --rename-head \ node-tap node-mongodb node-tap: bad description: Repository imported from https://anonscm.debian.org/git/pkg-javascript/node-tap.git/ Default branch is master kgb missing Tagpending missing node-mongodb: Default branch is master kgb missing # All perl-team packages: $ salsa check_repo --team-id=2666 --desc \ --kgb --irc-channel=debian-perl \ --tagpending \ --all (takes a very long time)
Bug#890594: salsa script ready to review
Le 29/10/2018 à 12:27, Raphael Hertzog a écrit : > On Sun, 28 Oct 2018, Xavier wrote: >> Mattia explained me dep14. I found a way to do it: create branch from >> master, update project to set default_branch to debian/master then >> remove master. It works as expected. >> >> $ salsa update_repo node-mongodb --group js-team --rename-head >> >> $ salsa update_repo --all --rename-head --no-fail # all user projects >> >> Manpage updated: >> https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L339 > > Nice, thanks! > > Now, this tool is really powerful and one could be wary of breaking > things. It would be nice if there was a "--no-act" option that would not > change anything but only display what would be done. > > It could be useful to see what repositories are currently not following > the usual rules and double check that we really want to override their > current configuration. Done: now you have a "check_repo" command that reports bad configured repositories without modifying anything. https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L117 > Cheers, Regards, Xavier
Bug#890594: salsa script ready to review
On Sun, 28 Oct 2018, Xavier wrote: > Mattia explained me dep14. I found a way to do it: create branch from > master, update project to set default_branch to debian/master then > remove master. It works as expected. > > $ salsa update_repo node-mongodb --group js-team --rename-head > > $ salsa update_repo --all --rename-head --no-fail # all user projects > > Manpage updated: > https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L339 Nice, thanks! Now, this tool is really powerful and one could be wary of breaking things. It would be nice if there was a "--no-act" option that would not change anything but only display what would be done. It could be useful to see what repositories are currently not following the usual rules and double check that we really want to override their current configuration. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 28/10/2018 à 09:47, Xavier a écrit : > Le 28/10/2018 à 08:43, Xavier a écrit : >> Le 27/10/2018 à 14:24, Xavier a écrit : >>> Le 25/10/2018 à 11:33, Raphael Hertzog a écrit : Hi, >>> >>> Hello, >>> On Fri, 19 Oct 2018, Xavier wrote: > first version of salsa script is ready to review. Documentation is here: > https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl Thanks for working on this! >>> >>> You're welcome ;-) >>> My first comment is that we should not have to work with numerical identifiers. I want to modify the group "pkg-security-team" and not the team-id 1234. The tool should do the lookup for me. I don't have to know internal identifiers. >>> >>> Fixed: you can use --group. If more than one group has this name, you'll >>> be invited to use --group-id (results are displayed on this error). >>> >>> I added a cache file to minimize Gitlab queries (with a purge_cache >>> command). Default to ~/.cache/salsa.json >>> Furthermore "team-id" is really not consistent with the the gitlab vocabulary. They use "group id". >>> >>> Fixed: team and group is now equivalent (commands and options) >>> > - manage repos The set of commands that you propose are really tailored to the current hooks that we are using. This is convenient to use but it is not future-proof. I would rather have a configuration file describing all parameters that we want to see configured and be able to pass that configuration file to the tool. >>> >>> I'm going to add a --conf-file for this >> >> Done >> I would love also if the tool could enforce things like: - rename "master" into "debian/master" (when debian/master doesn't exist) >> >> I don't understand this proposition. > > This is the last point to do but I need to understand it. Mattia explained me dep14. I found a way to do it: create branch from master, update project to set default_branch to debian/master then remove master. It works as expected. $ salsa update_repo node-mongodb --group js-team --rename-head $ salsa update_repo --all --rename-head --no-fail # all user projects Manpage updated: https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl#L339 - define protected branches (including the possibility to disable all protections) >> >> This is my next step ;-) > > Done > > Of course, most of options can be set in ~/.devscripts. Example: > > SALSA_KGB=yes > SALSA_IRC_CHANNEL=debian-perl > SALSA_TEAM_ID=2665 > SALSA_DESC=yes > SALSA_DESC_PATTERN="Perl team package %p" > SALSA_TOKEN=abcdef This looks like a misfeature. It's too easy to forget about those and apply unwanted settings to other repositories. >>> >>> I'm going to add a --conf-file for this >> >> Done >> Only SALSA_TOKEN is fine (and even there, this is private data and it might be better handled through some other mechanism?). >>> >>> Fixed, you can choose SALSA_TOKEN or SALSA_TOKEN_FILE or --token or >>> --token-file >>> > QUESTION 1: is "salsa" a good name? Something more explicit would be better. I'm not good at names. Maybe "debsalsa" or "salsa-configure". But then it could be designed as a generic gitlab configuration tool and you could entirely avoid the salsa marker in the name... >>> >>> gitlab-cfg ? Is it a good idea to ask to debian-devel@l.d.o ? >>> > QUESTION 2: I think --all should fail unless current user is owner, > isn't it? You might want to have some interactive confirmation. "You are about to modify the configuration of 245 repositories. Do you want to continue ?". But otherwise I don't think that you need to handle any access control, let gitlab take care of this part. >>> >>> Fixed: question unless --yes >>> Cheers, >>> >>> Regards, >>> Xavier >
Bug#890594: salsa script ready to review
Le 28/10/2018 à 08:43, Xavier a écrit : > Le 27/10/2018 à 14:24, Xavier a écrit : >> Le 25/10/2018 à 11:33, Raphael Hertzog a écrit : >>> Hi, >> >> Hello, >> >>> On Fri, 19 Oct 2018, Xavier wrote: first version of salsa script is ready to review. Documentation is here: https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl >>> >>> Thanks for working on this! >> >> You're welcome ;-) >> >>> My first comment is that we should not have to work with numerical >>> identifiers. I want to modify the group "pkg-security-team" and not >>> the team-id 1234. The tool should do the lookup for me. I don't have to >>> know internal identifiers. >> >> Fixed: you can use --group. If more than one group has this name, you'll >> be invited to use --group-id (results are displayed on this error). >> >> I added a cache file to minimize Gitlab queries (with a purge_cache >> command). Default to ~/.cache/salsa.json >> >>> Furthermore "team-id" is really not consistent with the the gitlab >>> vocabulary. They use "group id". >> >> Fixed: team and group is now equivalent (commands and options) >> - manage repos >>> >>> The set of commands that you propose are really tailored to the current >>> hooks that we are using. This is convenient to use but it is not >>> future-proof. I would rather have a configuration file describing all >>> parameters that we want to see configured and be able to pass that >>> configuration file to the tool. >> >> I'm going to add a --conf-file for this > > Done > >>> I would love also if the tool could enforce things like: >>> - rename "master" into "debian/master" (when debian/master doesn't exist) > > I don't understand this proposition. This is the last point to do but I need to understand it. >>> - define protected branches (including the possibility to disable all >>> protections) > > This is my next step ;-) Done Of course, most of options can be set in ~/.devscripts. Example: SALSA_KGB=yes SALSA_IRC_CHANNEL=debian-perl SALSA_TEAM_ID=2665 SALSA_DESC=yes SALSA_DESC_PATTERN="Perl team package %p" SALSA_TOKEN=abcdef >>> >>> This looks like a misfeature. It's too easy to forget about those and >>> apply unwanted settings to other repositories. >> >> I'm going to add a --conf-file for this > > Done > >>> Only SALSA_TOKEN is fine (and even there, this is private data and it >>> might be better handled through some other mechanism?). >> >> Fixed, you can choose SALSA_TOKEN or SALSA_TOKEN_FILE or --token or >> --token-file >> QUESTION 1: is "salsa" a good name? >>> >>> Something more explicit would be better. I'm not good at names. Maybe >>> "debsalsa" or "salsa-configure". But then it could be designed as a >>> generic gitlab configuration tool and you could entirely avoid the salsa >>> marker in the name... >> >> gitlab-cfg ? Is it a good idea to ask to debian-devel@l.d.o ? >> QUESTION 2: I think --all should fail unless current user is owner, isn't it? >>> >>> You might want to have some interactive confirmation. "You are about to >>> modify the configuration of 245 repositories. Do you want to continue ?". >>> >>> But otherwise I don't think that you need to handle any access control, >>> let gitlab take care of this part. >> >> Fixed: question unless --yes >> >>> Cheers, >> >> Regards, >> Xavier
Bug#890594: salsa script ready to review
Le 27/10/2018 à 14:24, Xavier a écrit : > Le 25/10/2018 à 11:33, Raphael Hertzog a écrit : >> Hi, > > Hello, > >> On Fri, 19 Oct 2018, Xavier wrote: >>> first version of salsa script is ready to review. Documentation is here: >>> https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl >> >> Thanks for working on this! > > You're welcome ;-) > >> My first comment is that we should not have to work with numerical >> identifiers. I want to modify the group "pkg-security-team" and not >> the team-id 1234. The tool should do the lookup for me. I don't have to >> know internal identifiers. > > Fixed: you can use --group. If more than one group has this name, you'll > be invited to use --group-id (results are displayed on this error). > > I added a cache file to minimize Gitlab queries (with a purge_cache > command). Default to ~/.cache/salsa.json > >> Furthermore "team-id" is really not consistent with the the gitlab >> vocabulary. They use "group id". > > Fixed: team and group is now equivalent (commands and options) > >>> - manage repos >> >> The set of commands that you propose are really tailored to the current >> hooks that we are using. This is convenient to use but it is not >> future-proof. I would rather have a configuration file describing all >> parameters that we want to see configured and be able to pass that >> configuration file to the tool. > > I'm going to add a --conf-file for this Done >> I would love also if the tool could enforce things like: >> - rename "master" into "debian/master" (when debian/master doesn't exist) I don't understand this proposition. >> - define protected branches (including the possibility to disable all >> protections) This is my next step ;-) >>> Of course, most of options can be set in ~/.devscripts. Example: >>> >>> SALSA_KGB=yes >>> SALSA_IRC_CHANNEL=debian-perl >>> SALSA_TEAM_ID=2665 >>> SALSA_DESC=yes >>> SALSA_DESC_PATTERN="Perl team package %p" >>> SALSA_TOKEN=abcdef >> >> This looks like a misfeature. It's too easy to forget about those and >> apply unwanted settings to other repositories. > > I'm going to add a --conf-file for this Done >> Only SALSA_TOKEN is fine (and even there, this is private data and it >> might be better handled through some other mechanism?). > > Fixed, you can choose SALSA_TOKEN or SALSA_TOKEN_FILE or --token or > --token-file > >>> QUESTION 1: is "salsa" a good name? >> >> Something more explicit would be better. I'm not good at names. Maybe >> "debsalsa" or "salsa-configure". But then it could be designed as a >> generic gitlab configuration tool and you could entirely avoid the salsa >> marker in the name... > > gitlab-cfg ? Is it a good idea to ask to debian-devel@l.d.o ? > >>> QUESTION 2: I think --all should fail unless current user is owner, >>> isn't it? >> >> You might want to have some interactive confirmation. "You are about to >> modify the configuration of 245 repositories. Do you want to continue ?". >> >> But otherwise I don't think that you need to handle any access control, >> let gitlab take care of this part. > > Fixed: question unless --yes > >> Cheers, > > Regards, > Xavier
Bug#890594: salsa script ready to review
Le 25/10/2018 à 11:33, Raphael Hertzog a écrit : > Hi, Hello, > On Fri, 19 Oct 2018, Xavier wrote: >> first version of salsa script is ready to review. Documentation is here: >> https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl > > Thanks for working on this! You're welcome ;-) > My first comment is that we should not have to work with numerical > identifiers. I want to modify the group "pkg-security-team" and not > the team-id 1234. The tool should do the lookup for me. I don't have to > know internal identifiers. Fixed: you can use --group. If more than one group has this name, you'll be invited to use --group-id (results are displayed on this error). I added a cache file to minimize Gitlab queries (with a purge_cache command). Default to ~/.cache/salsa.json > Furthermore "team-id" is really not consistent with the the gitlab > vocabulary. They use "group id". Fixed: team and group is now equivalent (commands and options) >> - manage repos > > The set of commands that you propose are really tailored to the current > hooks that we are using. This is convenient to use but it is not > future-proof. I would rather have a configuration file describing all > parameters that we want to see configured and be able to pass that > configuration file to the tool. I'm going to add a --conf-file for this > I would love also if the tool could enforce things like: > - rename "master" into "debian/master" (when debian/master doesn't exist) > - define protected branches (including the possibility to disable all > protections) I don't understand this part. You'll talking about "root" projects ? >> Of course, most of options can be set in ~/.devscripts. Example: >> >> SALSA_KGB=yes >> SALSA_IRC_CHANNEL=debian-perl >> SALSA_TEAM_ID=2665 >> SALSA_DESC=yes >> SALSA_DESC_PATTERN="Perl team package %p" >> SALSA_TOKEN=abcdef > > This looks like a misfeature. It's too easy to forget about those and > apply unwanted settings to other repositories. I'm going to add a --conf-file for this > Only SALSA_TOKEN is fine (and even there, this is private data and it > might be better handled through some other mechanism?). Fixed, you can choose SALSA_TOKEN or SALSA_TOKEN_FILE or --token or --token-file >> QUESTION 1: is "salsa" a good name? > > Something more explicit would be better. I'm not good at names. Maybe > "debsalsa" or "salsa-configure". But then it could be designed as a > generic gitlab configuration tool and you could entirely avoid the salsa > marker in the name... gitlab-cfg ? Is it a good idea to ask to debian-devel@l.d.o ? >> QUESTION 2: I think --all should fail unless current user is owner, >> isn't it? > > You might want to have some interactive confirmation. "You are about to > modify the configuration of 245 repositories. Do you want to continue ?". > > But otherwise I don't think that you need to handle any access control, > let gitlab take care of this part. Fixed: question unless --yes > Cheers, Regards, Xavier
Bug#890594: salsa script ready to review
Hi, On Fri, 19 Oct 2018, Xavier wrote: > first version of salsa script is ready to review. Documentation is here: > https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl Thanks for working on this! My first comment is that we should not have to work with numerical identifiers. I want to modify the group "pkg-security-team" and not the team-id 1234. The tool should do the lookup for me. I don't have to know internal identifiers. Furthermore "team-id" is really not consistent with the the gitlab vocabulary. They use "group id". > - manage repos The set of commands that you propose are really tailored to the current hooks that we are using. This is convenient to use but it is not future-proof. I would rather have a configuration file describing all parameters that we want to see configured and be able to pass that configuration file to the tool. I would love also if the tool could enforce things like: - rename "master" into "debian/master" (when debian/master doesn't exist) - define protected branches (including the possibility to disable all protections) > Of course, most of options can be set in ~/.devscripts. Example: > > SALSA_KGB=yes > SALSA_IRC_CHANNEL=debian-perl > SALSA_TEAM_ID=2665 > SALSA_DESC=yes > SALSA_DESC_PATTERN="Perl team package %p" > SALSA_TOKEN=abcdef This looks like a misfeature. It's too easy to forget about those and apply unwanted settings to other repositories. Only SALSA_TOKEN is fine (and even there, this is private data and it might be better handled through some other mechanism?). > QUESTION 1: is "salsa" a good name? Something more explicit would be better. I'm not good at names. Maybe "debsalsa" or "salsa-configure". But then it could be designed as a generic gitlab configuration tool and you could entirely avoid the salsa marker in the name... > QUESTION 2: I think --all should fail unless current user is owner, > isn't it? You might want to have some interactive confirmation. "You are about to modify the configuration of 245 repositories. Do you want to continue ?". But otherwise I don't think that you need to handle any access control, let gitlab take care of this part. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: https://www.freexian.com/services/debian-lts.html Learn to master Debian: https://debian-handbook.info/get/
Bug#890594: salsa script ready to review
Le 21/10/2018 à 16:26, gregor herrmann a écrit : > On Fri, 19 Oct 2018 11:07:02 +0200, Xavier wrote: > >> QUESTION 1: is "salsa" a good name? > > Not sure … one one hand it's a bit generic, on the other hand it's > nice and short and clear, and "bts" for example is similar. > (I don't have a better idea …) > >> QUESTION 2: I think --all should fail unless current user is owner, >> isn't it? > > I don't think this makes sense in a team, where the creator of a > project is the owner and later someone else (who is maintainer) wants > to change all repos. > > And also for packages in the debian group owner = salsa admins, so > noone else can make changes. OK, for now I disallowed --all only for "debian" main group (id=2). I think this makes no sense to configure kgb globally for this group. If needed, it is easy to modify this, but I think this can avoid errors in this widely used group >> @gregoa: you will be able to embed Devscripts::Salsa and build a >> Devscripts::Salsa::Config object to set values based on .dpt.conf >> instead of using .devscripts > > Nice :) > > Cheers, > gregor >
Bug#890594: salsa script ready to review
On Fri, 19 Oct 2018 11:07:02 +0200, Xavier wrote: > QUESTION 1: is "salsa" a good name? Not sure … one one hand it's a bit generic, on the other hand it's nice and short and clear, and "bts" for example is similar. (I don't have a better idea …) > QUESTION 2: I think --all should fail unless current user is owner, > isn't it? I don't think this makes sense in a team, where the creator of a project is the owner and later someone else (who is maintainer) wants to change all repos. And also for packages in the debian group owner = salsa admins, so noone else can make changes. > @gregoa: you will be able to embed Devscripts::Salsa and build a > Devscripts::Salsa::Config object to set values based on .dpt.conf > instead of using .devscripts Nice :) Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Featuring The Dubliners, The Fureys And Davey Arthur Etc.: Dublin Town, Brendan Grace signature.asc Description: Digital Signature
Bug#890594: salsa script ready to review
Hello, first version of salsa script is ready to review. Documentation is here: https://salsa.debian.org/yadd/devscripts/blob/devscripts-salsa-890594/scripts/salsa.pl Resume: - usage salsa - manage users in group: salsa --team-id=1234 add_user maintainer foobar salsa --team-id=1234 del_user foo salsa --team-id 1234 update_user owner foobar salsa --team-id 1234 team # list members salsa whoami - search id and other info salsa search_user yadd salsa search_team perl-team - manage repos salsa --user-id 1234 create_repo test salsa --team-id 123 --kgb --irc-channel=devscripts create_repo test salsa --team-id 123 --tagpending --desc --desc-pattern "Repo %p" \ create_repo test salsa --user-id 1234 push_repo test # create repo based on # test/debian/changelog name # and push content salsa --user-id 1234 create_repo ./test # same as push_repo salsa --team-id 1234 update_repo test --tagpending salsa --team-id 1234 update_repo --all --desc \ --desc-pattern "Perl Team package %p" salsa --user-id 1234 update_repo --all --desc # default value: # "Debian package %p" Of course, most of options can be set in ~/.devscripts. Example: SALSA_KGB=yes SALSA_IRC_CHANNEL=debian-perl SALSA_TEAM_ID=2665 SALSA_DESC=yes SALSA_DESC_PATTERN="Perl team package %p" SALSA_TOKEN=abcdef Then to enable IRC without changing description: salsa --team-id 1234 update_repo --all --no-desc Or to update description but not KGB hook: salsa --team-id 1234 update_repo --all --no-kgb Each command is a distinct little file in Devscripts::Salsa area, so I think contributions will be easy. QUESTION 1: is "salsa" a good name? QUESTION 2: I think --all should fail unless current user is owner, isn't it? @gregoa: you will be able to embed Devscripts::Salsa and build a Devscripts::Salsa::Config object to set values based on .dpt.conf instead of using .devscripts Cheers, Xavier