Skip to content

add a check for remote-id validity#387

Merged
arthurzam merged 1 commit intomasterfrom
remote-id-valid
May 25, 2022
Merged

add a check for remote-id validity#387
arthurzam merged 1 commit intomasterfrom
remote-id-valid

Conversation

@mgorny
Copy link
Copy Markdown
Contributor

@mgorny mgorny commented May 24, 2022

Add an auxiliary check for invalid remote-id values.  At the moment,
it primarily checks for the correct number of path components and some
obvious mistakes.  In the future, the regular expressions can be
improved to catch invalid user/project names as well.

This includes tests for common valid and invalid values for the current
set of remote-ids.  In addition to that, PkgMetadataXmlEmptyElement
check is extended to cover "empty <remote-id/>" path in the code.
Unfortunately, covering "unknown type" path is non-trivial as it
requires a remote-id type that is allowed by the XML Schema but not
covered by validation (which normally happens only when Schema is
updated).

This also requires bumping the minimal pkgcore version in order to grab
XML Schema updates (adding "osdn" remote type).
Current output
$ pkgcheck scan -k InvalidRemoteID
app-backup/deja-dup
  InvalidRemoteID: remote-id value 'https://gitlab.gnome.org/World/deja-dup' invalid for type='gitlab', expected: '{username}/[{group}/...]{repo}'

app-crypt/adcli
  InvalidRemoteID: remote-id value 'https://gitlab.freedesktop.org/realmd/adcli' invalid for type='gitlab', expected: '{username}/[{group}/...]{repo}'

app-emulation/wine-gecko
  InvalidRemoteID: remote-id value 'wine/wine-gecko' invalid for type='sourceforge', expected: '{project}'

app-text/pdfgrep
  InvalidRemoteID: remote-id value 'pdfgrep' invalid for type='gitlab', expected: '{username}/[{group}/...]{repo}'

dev-go/gopls
  InvalidRemoteID: remote-id value 'golang/tools/gopls' invalid for type='github', expected: '{username}/{project}'

dev-java/fec
  InvalidRemoteID: remote-id value 'onionnetworks' invalid for type='bitbucket', expected: '{username}/{project}'

dev-java/jackson-dataformat-yaml
  InvalidRemoteID: remote-id value 'https://github.com/FasterXML/jackson-dataformats-text/issues' invalid for type='github', expected: '{username}/{project}'

dev-java/junitparams
  InvalidRemoteID: remote-id value 'Pragmatists/JUnitParams/tags' invalid for type='github', expected: '{username}/{project}'

dev-libs/thrift
  InvalidRemoteID: remote-id value 'apache/thrift/' invalid for type='github', expected: '{username}/{project}'

dev-php/webmozart-assert
  InvalidRemoteID: remote-id value 'webmozart/' invalid for type='github', expected: '{username}/{project}'

dev-python/clikit
  InvalidRemoteID: remote-id value 'clikit' invalid for type='github', expected: '{username}/{project}'

dev-python/statsmodels
  InvalidRemoteID: remote-id value 'statsmodels' invalid for type='github', expected: '{username}/{project}'

dev-python/translate-toolkit
  InvalidRemoteID: remote-id value 'translate' invalid for type='github', expected: '{username}/{project}'

dev-scheme/racket
  InvalidRemoteID: remote-id value 'racket' invalid for type='github', expected: '{username}/{project}'

media-radio/js8call
  InvalidRemoteID: remote-id value 'js8call' invalid for type='bitbucket', expected: '{username}/{project}'

net-analyzer/gvm
  InvalidRemoteID: remote-id value 'greenbone' invalid for type='github', expected: '{username}/{project}'

sci-libs/symmetrica
  InvalidRemoteID: remote-id value 'https://gitlab.com/sagemath/symmetrica' invalid for type='gitlab', expected: '{username}/[{group}/...]{repo}'

x11-themes/ubuntu-wallpapers
  InvalidRemoteID: remote-id value 'ubuntu/+source/ubuntu-wallpapers' invalid for type='launchpad', expected: '{project}'

@codecov
Copy link
Copy Markdown

codecov Bot commented May 24, 2022

Codecov Report

Merging #387 (cf29c73) into master (a054f7f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head cf29c73 differs from pull request most recent head 97370ba. Consider uploading reports for the commit 97370ba to get more accurate results

@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   95.66%   95.67%   +0.01%     
==========================================
  Files          55       55              
  Lines        7543     7565      +22     
  Branches     1838     1842       +4     
==========================================
+ Hits         7216     7238      +22     
  Misses        203      203              
  Partials      124      124              
Impacted Files Coverage Δ
src/pkgcheck/checks/metadata_xml.py 96.69% <100.00%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a054f7f...97370ba. Read the comment docs.

@mgorny mgorny force-pushed the remote-id-valid branch 2 times, most recently from 047e688 to c73138a Compare May 24, 2022 19:40
Comment thread testdata/repos/standalone/PackageMetadataXmlCheck/InvalidRemoteID/metadata.xml Outdated
@mgorny mgorny force-pushed the remote-id-valid branch from c73138a to dc41283 Compare May 25, 2022 04:59
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented May 25, 2022

  • figured out how to make gentoo work with a regexp (without extra class)
  • added check for lp: prefix in launchpad names

Copy link
Copy Markdown
Member

@arthurzam arthurzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also add a testdata/repos/standalone/PackageMetadataXmlCheck/ValidRemoteID/metadata.xml which will hold valid remotes (one for each remote).

Comment thread src/pkgcheck/checks/metadata_xml.py Outdated
Comment thread testdata/repos/standalone/PackageMetadataXmlCheck/InvalidRemoteID/metadata.xml Outdated
@mgorny mgorny force-pushed the remote-id-valid branch from dc41283 to 0f0fb22 Compare May 25, 2022 09:30
@mgorny mgorny changed the title add a check for remote-id validity (WIP) add a check for remote-id validity May 25, 2022
@mgorny mgorny force-pushed the remote-id-valid branch 4 times, most recently from 7ac4aca to 455e3f6 Compare May 25, 2022 11:03
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented May 25, 2022

Ok, I don't think we can cover the KeyError case as that would require having an XML Schema entry without a validator.

@mgorny mgorny force-pushed the remote-id-valid branch from 455e3f6 to 1243246 Compare May 25, 2022 11:10
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented May 25, 2022

Ah, right, need to bump pkgcore dep to get fresh XML Schema.

@mgorny mgorny force-pushed the remote-id-valid branch from 1243246 to cf29c73 Compare May 25, 2022 12:35
@mgorny mgorny requested a review from arthurzam May 25, 2022 12:35
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented May 25, 2022

Ok, I think it's ready now.

Copy link
Copy Markdown
Member

@arthurzam arthurzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thank you!

Add an auxiliary check for invalid remote-id values.  At the moment,
it primarily checks for the correct number of path components and some
obvious mistakes.  In the future, the regular expressions can be
improved to catch invalid user/project names as well.

This includes tests for common valid and invalid values for the current
set of remote-ids.  In addition to that, PkgMetadataXmlEmptyElement
check is extended to cover "empty <remote-id/>" path in the code.
Unfortunately, covering "unknown type" path is non-trivial as it
requires a remote-id type that is allowed by the XML Schema but not
covered by validation (which normally happens only when Schema is
updated).

This also requires bumping the minimal pkgcore version in order to grab
XML Schema updates (adding "osdn" remote type).
@mgorny mgorny force-pushed the remote-id-valid branch from cf29c73 to 97370ba Compare May 25, 2022 19:51
@arthurzam arthurzam merged commit 97370ba into master May 25, 2022
@arthurzam arthurzam deleted the remote-id-valid branch May 25, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants