Details

Reviewers

vlorentz

Maniphest Tasks

T4376: Metadata Indexer for Pub (pubspec.yaml)

Commits

rDCIDXfa67b73d6a88: metadata_dictionary: Add mappings for pubspec.yaml

Summary

Fixes T4376

Test Plan

Write a test case for the basic pub mapping

	   Test edge cases with examples (maybe from sourcegraph)

Diff Detail

Repository

rDCIDX Metadata indexer

Branch

Pub-Mapping

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 30311
Build 47385: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 47384: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	4,765 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.indexer.tests.test_cli::test_cli_mapping_list cli_runner = <click.testing.CliRunner object at 0x7f22d1ec67f0> swh_config = '/tmp/pytest-of-jenkins/pytest-0/test_cli_mapping_list0/indexer.yml'
	4 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.indexer.codemeta::swh.indexer.codemeta.merge_values
	2 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.indexer.metadata_dictionary.github::swh.indexer.metadata_dictionary.github.GitHubMapping.normalize_license
	1 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.indexer.metadata_dictionary.github::swh.indexer.metadata_dictionary.github.GitHubMapping.translate_forks_count
	1 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.indexer.metadata_dictionary.maven::swh.indexer.metadata_dictionary.maven.MavenMapping.normalize_groupId
		View Full Test Results (1 Failed · 368 Passed · 15 Skipped)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Squash commits

Build is green

Patch application report for D8079 (id=29228)

Rebasing onto c2742b5b75...

First, rewinding head to replay your work on top of it...
Applying: metadata_dictionary: Add mappings for pubspec.yaml

Changes applied before test

commit bb56919c89c6953421789ba72bda6b4f61610adf
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/350/ for more details.

Harbormaster completed remote builds in B30316: Diff 29228.Jul 7 2022, 8:32 PM

Why is so much code commented out?

swh/indexer/metadata_dictionary/pub.py
41–53	Move this to a new `base.YamlMapping` to deduplicate the code with `CffMapping`
swh/indexer/tests/test_cli.py
104	keep this entry instead of adding `\n` to the other one

This revision now requires changes to proceed.Jul 7 2022, 9:45 PM

Adds base class for mapping Yaml files

Made the requested changes. The 2 methods that are in PubMapping are also deduplicated from CffMapping. However it felt intuitive to leave them that way (and not include them in YamlMapping).

Am I wrong? I'll change it if so.

Build is green

Patch application report for D8079 (id=29230)

Rebasing onto c2742b5b75...

First, rewinding head to replay your work on top of it...
Applying: metadata_dictionary: Add mappings for pubspec.yaml
Using index info to reconstruct a base tree...
M	swh/indexer/metadata_dictionary/cff.py
Falling back to patching base and 3-way merge...
Auto-merging swh/indexer/metadata_dictionary/cff.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/cff.py
Patch failed at 0001 metadata_dictionary: Add mappings for pubspec.yaml

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!

Could not rebase; Attempt merge onto c2742b5b75...

Already up to date.

Changes applied before test

commit 94f4a5f6cb5aa2e95f7abdf9c51712ee31cbbae8
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/351/ for more details.

Harbormaster completed remote builds in B30317: Diff 29230.Jul 7 2022, 11:13 PM

Thanks. Some more comments now that we have the basics covered:

Hmm, I wonder what we could translate the pubspec documentation key to. codemeta:readme and codemeta:buildInstructions both feel slightly off, and schema:documentation seems to be only for web APIs for some reason...

Also, could you rename the mapping to call this "pubspec"? "pub" alone is rather ambiguous. And probably rename the file to "dart.py", as file names tend to describe communities rather than formats (hypothetically, we would have all three mappings for PKG-INFO/setup.cfg/pyproject.toml in python.py for example)

swh/indexer/data/pub.csv
4	could translate this from the `platforms` field
21	didn't you say there is also an `authors` field?
34	I this field documented? I don't see it on https://dart.dev/tools/pub/pubspec
swh/indexer/metadata_dictionary/cff.py
33–35 ↗	(On Diff #29230)	please rebase before submitting diffs; this code was simplified; so you can simplify the new base YamlMapping similarly
swh/indexer/metadata_dictionary/pub.py
1	only 2022 as far as I can tell

This revision now requires changes to proceed.Jul 8 2022, 8:36 AM

VickyMerzOwn marked 5 inline comments as done.Jul 8 2022, 10:21 AM

VickyMerzOwn added inline comments.

swh/indexer/data/pub.csv
21	Yeah, but both `author` and `authors` can be mapped to `author`. I forgot to add it. Will look into it now.
34	You're right, my bad. There has been a brief discussion on this: https://github.com/dart-lang/pub-dev/issues/5535 How come other libraries have a license field then?
swh/indexer/metadata_dictionary/pub.py
1	Sorry, I didn't understand what you mean. Should I remove "The Software Heritage developers"?

Change mapping name
Rebase

vlorentz added inline comments.Jul 8 2022, 10:29 AM

swh/indexer/data/pub.csv
34	probably because some project boilerplate generator writes it and/or people copy this from each other without checking the documentation
swh/indexer/metadata_dictionary/pub.py
1	I meant replacing "2017-2022" with only "2022" (and in test_pub.py too)

Exclude license field from mapping

Build is green

Patch application report for D8079 (id=29232)

Rebasing onto c2742b5b75...

Current branch diff-target is up to date.

Changes applied before test

commit 68170dda98fda949d2988743838140680afe6789
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/352/ for more details.

Harbormaster completed remote builds in B30318: Diff 29232.Jul 8 2022, 10:32 AM

vlorentz added inline comments.Jul 8 2022, 10:35 AM

swh/indexer/data/pub.csv
34	anyway, it's worth adding given the popularity. Please check people only use it with SPDX id though; and document it in dart.py (a one-line comment is probably enough)

Build has FAILED

Patch application report for D8079 (id=29233)

Rebasing onto c2742b5b75...

Current branch diff-target is up to date.

Changes applied before test

commit efaf1d810d0a3d9aff5ee1e534ee64af9146bd93
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/353/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/353/console

Harbormaster failed remote builds in B30319: Diff 29233!Jul 8 2022, 10:37 AM

VickyMerzOwn marked 2 inline comments as done.Jul 8 2022, 10:52 AM

VickyMerzOwn added inline comments.

swh/indexer/data/pub.csv
34	Would you like that I import the table from here: https://github.com/spdx/license-list-data/blob/master/licenses.md Then whenever a license field is present, it verifies if the value is in the table and only then adds it?

Adds license field back

Build is green

Patch application report for D8079 (id=29236)

Rebasing onto c2742b5b75...

Current branch diff-target is up to date.

Changes applied before test

commit 6ac7d51386b6f64b2b4972e10d4441525fcbb2b6
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/354/ for more details.

Harbormaster completed remote builds in B30321: Diff 29236.Jul 8 2022, 11:20 AM

vlorentz added inline comments.Jul 8 2022, 2:20 PM

swh/indexer/data/pub.csv
34	No, this table changes too frequently. I meant you should check that people use SPDX ids in practice

VickyMerzOwn added inline comments.Jul 8 2022, 2:27 PM

swh/indexer/data/pub.csv
34	Just got another idea. Even if there isn't a license field, the first line of the license file gives the type. Can I use that or is it diverging a lot from using mappings to translate metadata?

vlorentz added inline comments.Jul 8 2022, 3:21 PM

swh/indexer/data/pub.csv
34	it's actually more complicated than this, but possible with tools like fossology. It will need to be a separate mapping, though.

Normalize author/authors field(s)

Build is green

Patch application report for D8079 (id=29265)

Rebasing onto c2742b5b75...

Current branch diff-target is up to date.

Changes applied before test

commit 381cf1d53ab595fac7483b5621829397f91696ce
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/355/ for more details.

Harbormaster completed remote builds in B30351: Diff 29265.Jul 9 2022, 10:35 AM

Could you add a test with both author and authors?

swh/indexer/metadata_dictionary/dart.py
53–59 ↗	(On Diff #29265)	str.rsplit takes a `maxsplit` argument, so you can simplify your code like above. However, neither variant supports authors hiding their email address, and you should either add support for that, or make sure this never actually happens. (To implement it without making the code too convoluted, you can use this regexp: `(?P<name>.?)( <(?P<email>.)>)?`)
swh/indexer/tests/metadata_dictionary/test_pubspec.py
1 ↗	(On Diff #29265)	you should rename this file to match the other one
79 ↗	(On Diff #29265)	avoid real domain names in tests; use those defined in RFC6761 instead, such as "example.org"
89–90 ↗	(On Diff #29265)	same here

This revision now requires changes to proceed.Jul 11 2022, 10:02 AM

Changes test filename to test_dart.py
Checks for presence of email while normalizing author

Build is green

Patch application report for D8079 (id=29298)

Rebasing onto 2dd2be9c1b...

First, rewinding head to replay your work on top of it...
Applying: metadata_dictionary: Add mappings for pubspec.yaml
Using index info to reconstruct a base tree...
M	swh/indexer/metadata_dictionary/__init__.py
M	swh/indexer/metadata_dictionary/base.py
M	swh/indexer/metadata_dictionary/cff.py
Falling back to patching base and 3-way merge...
Auto-merging swh/indexer/metadata_dictionary/cff.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/cff.py
Auto-merging swh/indexer/metadata_dictionary/base.py
Auto-merging swh/indexer/metadata_dictionary/__init__.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/__init__.py
Patch failed at 0001 metadata_dictionary: Add mappings for pubspec.yaml

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!

Could not rebase; Attempt merge onto 2dd2be9c1b...

Already up to date.

Changes applied before test

commit 8e800e2f2ea7568b43a8ab2cfac58a329cb47c2f
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/357/ for more details.

Harbormaster completed remote builds in B30383: Diff 29298.Jul 12 2022, 12:32 AM

VickyMerzOwn added inline comments.Jul 12 2022, 1:19 AM

swh/indexer/metadata_dictionary/dart.py
53–59 ↗	(On Diff #29265)	Checking for "@" in the string is like a check if there is an email address in the string.

don't forget this:

In D8079#210852, @vlorentz wrote:

Could you add a test with both author and authors?

swh/indexer/metadata_dictionary/dart.py
53–59 ↗	(On Diff #29265)	It is not guaranteed. These files are untrusted input, they need to be fully validated to prevent crashes.

vlorentz requested changes to this revision.Jul 12 2022, 2:31 PM

This revision now requires changes to proceed.Jul 12 2022, 2:31 PM

In D8079#211132, @vlorentz wrote:

don't forget this:

In D8079#210852, @vlorentz wrote:

Could you add a test with both author and authors?

I think either of the fields will be present but not both.

Not sure if this query is accurate but it gives 0 results: https://sourcegraph.com/search?q=context:global+author+authors+file:pubspec.yaml&patternType=literal

https://sourcegraph.com/search?q=context:global+author+version+file:pubspec.yaml&patternType=literal also returns no result, because it searches for the literal string author version (or author authors in your example), see https://docs.sourcegraph.com/code_search/reference/queries#literal-search-default

So this is not conclusive

Verifies author value with regex
Adds test for files containing both 'author' and 'authors' fields

Build is green

Patch application report for D8079 (id=29326)

Rebasing onto 2dd2be9c1b...

First, rewinding head to replay your work on top of it...
Applying: metadata_dictionary: Add mappings for pubspec.yaml
Using index info to reconstruct a base tree...
M	swh/indexer/metadata_dictionary/__init__.py
M	swh/indexer/metadata_dictionary/base.py
M	swh/indexer/metadata_dictionary/cff.py
Falling back to patching base and 3-way merge...
Auto-merging swh/indexer/metadata_dictionary/cff.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/cff.py
Auto-merging swh/indexer/metadata_dictionary/base.py
Auto-merging swh/indexer/metadata_dictionary/__init__.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/__init__.py
Patch failed at 0001 metadata_dictionary: Add mappings for pubspec.yaml

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!

Could not rebase; Attempt merge onto 2dd2be9c1b...

Already up to date.

Changes applied before test

commit 3f72e93f35d9f8de093e1d9af9a46e38b42a52b9
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/358/ for more details.

Harbormaster completed remote builds in B30406: Diff 29326.Jul 12 2022, 4:14 PM

vlorentz added inline comments.Jul 12 2022, 5:33 PM

swh/indexer/metadata_dictionary/dart.py
56–57 ↗	(On Diff #29326)	use the regexp's capture groups, instead of parsing twice

Regex Capture groups

Build is green

Patch application report for D8079 (id=29333)

Rebasing onto 2dd2be9c1b...

First, rewinding head to replay your work on top of it...
Applying: metadata_dictionary: Add mappings for pubspec.yaml
Using index info to reconstruct a base tree...
M	swh/indexer/metadata_dictionary/__init__.py
M	swh/indexer/metadata_dictionary/base.py
M	swh/indexer/metadata_dictionary/cff.py
Falling back to patching base and 3-way merge...
Auto-merging swh/indexer/metadata_dictionary/cff.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/cff.py
Auto-merging swh/indexer/metadata_dictionary/base.py
Auto-merging swh/indexer/metadata_dictionary/__init__.py
CONFLICT (content): Merge conflict in swh/indexer/metadata_dictionary/__init__.py
Patch failed at 0001 metadata_dictionary: Add mappings for pubspec.yaml

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!

Could not rebase; Attempt merge onto 2dd2be9c1b...

Already up to date.

Changes applied before test

commit ffa4e5af7fdb1bd4f43d1ebb36ae6c48aa74ac5b
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 12 22:25:05 2022 +0530

    Capture groups

commit 3f72e93f35d9f8de093e1d9af9a46e38b42a52b9
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/359/ for more details.

Harbormaster completed remote builds in B30411: Diff 29333.Jul 12 2022, 7:06 PM

thanks. one last thing:

swh/indexer/metadata_dictionary/dart.py
54 ↗	(On Diff #29333)	redundant, `None` is not an instance of `str`

This revision now requires changes to proceed.Jul 12 2022, 7:20 PM

Remove redundant check that input string is not Nonetype
Rebase

Build is green

Patch application report for D8079 (id=29334)

Rebasing onto 2dd2be9c1b...

Current branch diff-target is up to date.

Changes applied before test

commit fa67b73d6a88461e76601c3b61ee6d33ba5a2c93
Author: Satvik Vemuganti <vemugantisesha@iitbhilai.ac.in>
Date:   Tue Jul 5 14:33:19 2022 +0530

    metadata_dictionary: Add mappings for pubspec.yaml

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/360/ for more details.

Harbormaster completed remote builds in B30412: Diff 29334.Jul 12 2022, 7:49 PM

vlorentz accepted this revision.Jul 12 2022, 9:56 PM

This revision is now accepted and ready to land.Jul 12 2022, 9:56 PM

Closed by commit rDCIDXfa67b73d6a88: metadata_dictionary: Add mappings for pubspec.yaml (authored by VickyMerzOwn). · Explain WhyJul 12 2022, 11:15 PM

This revision was automatically updated to reflect the committed changes.

VickyMerzOwn added a commit: rDCIDXfa67b73d6a88: metadata_dictionary: Add mappings for pubspec.yaml.

Metadata Indexer for Pub (pubspec.yaml)
ClosedPublic
Actions

Details

Diff Detail

Unit TestsFailed
View All

Event Timeline

Patch application report for D8079 (id=29228)

Changes applied before test

Patch application report for D8079 (id=29230)

Changes applied before test

Patch application report for D8079 (id=29232)

Changes applied before test

Patch application report for D8079 (id=29233)

Changes applied before test

Patch application report for D8079 (id=29236)

Changes applied before test

Patch application report for D8079 (id=29265)

Changes applied before test

Patch application report for D8079 (id=29298)

Changes applied before test

Patch application report for D8079 (id=29326)

Changes applied before test

Patch application report for D8079 (id=29333)

Changes applied before test

Patch application report for D8079 (id=29334)

Changes applied before test

Revision Contents
Changeset List

Diff 29223

swh/indexer/data/pub.csv

swh/indexer/metadata_dictionary/init.py

swh/indexer/metadata_dictionary/pub.py

swh/indexer/tests/metadata_dictionary/test_pub.py

swh/indexer/tests/test_cli.py

Metadata Indexer for Pub (pubspec.yaml)ClosedPublicActions

Details

Diff Detail

Unit TestsFailedView All

Event Timeline

Patch application report for D8079 (id=29228)

Changes applied before test

Patch application report for D8079 (id=29230)

Changes applied before test

Patch application report for D8079 (id=29232)

Changes applied before test

Patch application report for D8079 (id=29233)

Changes applied before test

Patch application report for D8079 (id=29236)

Changes applied before test

Patch application report for D8079 (id=29265)

Changes applied before test

Patch application report for D8079 (id=29298)

Changes applied before test

Patch application report for D8079 (id=29326)

Changes applied before test

Patch application report for D8079 (id=29333)

Changes applied before test

Patch application report for D8079 (id=29334)

Changes applied before test

Revision ContentsChangeset List

Diff 29223

swh/indexer/data/pub.csv

swh/indexer/metadata_dictionary/__init__.py

swh/indexer/metadata_dictionary/pub.py

swh/indexer/tests/metadata_dictionary/test_pub.py

swh/indexer/tests/test_cli.py

Metadata Indexer for Pub (pubspec.yaml)
ClosedPublic
Actions

Unit TestsFailed
View All

Revision Contents
Changeset List

swh/indexer/metadata_dictionary/init.py