Fix invalid license yaml files by resolving duplicated keys

[fangxlmr]

Fixes #2655.

Tasks

• Reviewed contribution guidelines
• PR is descriptively titled 📑 and links the original issue above 🔗
• Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
  Run tests locally to check for errors.
• Commits are in uniquely-named feature branch and has no merge conflicts 📁

[fangxlmr]

CC @pombredanne

[pombredanne]

Thank you for this... but I do not understand what is the problem you are trying to fix. Furthermore, why are you removing notes that are likely valuable?

[KaitoHH]

Thank you for this... but I do not understand what is the problem you are trying to fix. Furthermore, why are you removing notes that are likely valuable?

Duplicate key is illegal according to the latest YAML spec. Using other libraries to parse these YAML files, for example, go-yaml/yaml in golang, will cause illegal YAML errors, which causes lots of trouble.

[fangxlmr]

FYI. saneyaml allows duplicated keys. From experiment, the rule is "Last one wins".

[fangxlmr]

Furthermore, why are you removing notes that are likely valuable?

Most notes I have removed is EXACTLY duplicted if you take a glance on the context around them, except for ecos.yml and ekioh.yml.

And as notes in ecos.yml, it's replaced by ecos-exception-2.0. Do you want me to merge those two notes together? @pombredanne

[pombredanne]

@fangxlmr @KaitoHH ah... I got it! great catch!
I have a few nit pickings for your consideration.

[pombredanne]

@fangxlmr re:

FYI. saneyaml allows duplicated keys. From experiment, the rule is "Last one wins".

Yes, and it can also raise errors when there are duplicates... See
https://github.com/nexB/saneyaml/blob/f35baebf2da0fc161f397ff5a9d69d09fd3f4078/src/saneyaml.py#L50

IMHO we should also enforce this by using saneyaml.load(allow_duplicate_keys=False) here https://github.com/nexB/scancode-toolkit/blob/eee864090717611f79e178f3b4f642d108951e68/src/licensedcode/models.py#L261 and there https://github.com/nexB/scancode-toolkit/blob/eee864090717611f79e178f3b4f642d108951e68/src/licensedcode/models.py#L1231 otherwise there surely will be regressions

Just curious: how did you find out about these?

[fangxlmr]

@fangxlmr re:

FYI. saneyaml allows duplicated keys. From experiment, the rule is "Last one wins".

Yes, and it can also raise errors when there are duplicates... See https://github.com/nexB/saneyaml/blob/f35baebf2da0fc161f397ff5a9d69d09fd3f4078/src/saneyaml.py#L50

IMHO we should also enforce this by using saneyaml.load(allow_duplicate_keys=False) here

https://github.com/nexB/scancode-toolkit/blob/eee864090717611f79e178f3b4f642d108951e68/src/licensedcode/models.py#L261

and there
https://github.com/nexB/scancode-toolkit/blob/eee864090717611f79e178f3b4f642d108951e68/src/licensedcode/models.py#L1231

otherwise there surely will be regressions

Sure. During resolving dup fields in license yamls, I also found out this mistake occurs in one rule yaml as well. Not suprised though.

Just curious: how did you find out about these?

Sharp eyes :P

[fangxlmr]

@pombredanne Done.

[pombredanne]

This is clean and clear. Looking great. Thank you ++ 🙇
Merging