Do not fail to scan broken package manifests (such as broken test files) [was: Successful scan outputs to stderr]

[geneh]

Repro steps (scancode-toolkit-2.9.0b1):
git clone [email protected]:benmosher/eslint-plugin-import.git
Run
/Users/gene/Downloads/scancode-toolkit-2.9.0b1/scancode --copyright --license --info --package --license-diag --only-findings --strip-root --timeout 1000 -n 2 --json-pp ./output290-eslint-plugin-import.json ./eslint-plugin-import 2> err.txt

The scanning results are successfully saved in the output file but the results are redirected into stderr instead of stdout, probably because of the warning below (eslint-plugin-import/tests/files/internal-modules/package.json & eslint-plugin-import/tests/files/with-syntax-error/package.json are empty).

$ cat err.txt 
Setup plugins...
Collect file inventory...
Scan files for: info, licenses, copyrights, packages with 2 process(es)...
Scanning done.
Some files failed to scan properly:
Path: eslint-plugin-import/tests/files/internal-modules/package.json
Path: eslint-plugin-import/tests/files/with-syntax-error/package.json
Summary:        info, copyrights, licenses, packages with 2 process(es)
Errors count:   0
Scan Speed:     30.15 files/sec. 41.79 KB/sec.
Initial counts: 429 resource(s): 345 file(s) and 84 directorie(s) 
Final counts:   20 resource(s): 20 file(s) and 0 directorie(s) for 63.46 KB
Timings:
  setup_scan:licenses: 2.58s
  setup: 2.58s
  inventory: 0.10s
  scan: 11.44s
  total: 14.17s

[pombredanne]

@geneh Thanks for the report.
The scan summary is always displayed on stderr and never to stdout. This is made so such that you can redirect the stdout scan results (when using the - special file) to a file and not pollute the structured data with the summary and other CLI progress report details.

Were you expecting to get the content to err.txt to stdout instead?

Also I guess than an empty package.json should be silently ignored rather than triggering an error

[geneh]

Thanks, @pombredanne. Yes, I think it should be silently ignored because we're failing the process on our end if an error is triggered. stderr vs stdout is fine with me.

[pombredanne]

good, I will keep this open until this is fixed.
Note also that eventually #426 which is about classification aka. facets in clearlydefiined speak should also make this easier as eventually there are tons of weird stuff in tests files across all packages. Eespecially when the packages are either package-related tools, linters, code analyzers and license/copyright-related tools: some test files are often meant to be broken to support tests. A funny problem of its own.

[pombredanne]

@geneh Here is what I suggest we do here:

1. if a package.json (or in general a package manifest) is broken or empty and cannot be parsed alright, this would not be returned as an error at all, and silently ignored as being a package manifest.
2. we could also report some tag stating that this is likely a package manifest in anycase, but in this case without any collected metadata (since it fails to parse)
3. the file would still be scanned for license/copyrights but as a plain text file rather than a structured manifest

[geneh]

@pombredanne It sounds good to me.

[pombredanne]

Good! this will come up as part of the #275 work and branch/PR #823

[pombredanne]

Fixed in develop and available also in the latest released tags. Thanks!
I am keeping this open as a reminder to push updates on the CD side to use the latest ScanCode version soon.