Lockifest manifests generate errors when `--no-generation` set

[maxrake]

Overview

Attempting to parse a Python lockifest with the --no-generation option results in an unhandled parsing error when the lockifest is a manifest.

How To Reproduce

Steps to reproduce this behavior:

Details (click to expand)

``` # Show the current released version ❯ phylum --version phylum v5.8.1 # Create a basic Python lockifest manifest ❯ cat requirements.txt pyyaml # Show the current released version works as expected ❯ phylum parse --lockfile-type pip ./requirements.txt Generating lockfile for manifest "requirements.txt" using Pip… [ { "name": "PyYAML", "version": "6.0.1", "type": "pypi", "lockfile": "requirements.txt" } ] # Update to the latest release candidate ❯ phylum update --prerelease phylum-cli installer OK Copied completions to /Users/maxrake/.local/share/phylum/completions Installing extension bundle... ✅ Extension bundle already installed, nothing to do Installing extension cargo... ✅ Extension cargo already installed, nothing to do Installing extension npm... ✅ Extension npm already installed, nothing to do Installing extension pip... ✅ Extension pip already installed, nothing to do Installing extension poetry... ✅ Extension poetry already installed, nothing to do Installing extension yarn... ✅ Extension yarn already installed, nothing to do OK Installed default extensions OK Completions are enabled for bash. OK Completions are enabled for zsh. OK Successfully installed phylum. Source your /Users/maxrake/.zshrc file, add /Users/maxrake/.local/bin to your $PATH variable, or log in to a new terminal in order to make `phylum` available. ✅ Successfully updated to v5.9.0-rc1! # Show the RC version ❯ phylum --version phylum v5.9.0-rc1 # Show the RC version works when `--no-generation` is NOT specified ❯ phylum parse --lockfile-type pip ./requirements.txt Generating lockfile for manifest "requirements.txt" using Pip… [ { "name": "PyYAML", "version": "6.0.1", "type": "pypi", "lockfile": "requirements.txt" } ] # Show the RC version fails when `--no-generation` IS specified ❯ phylum parse --lockfile-type pip --no-generation ./requirements.txt ❗ Error: could not parse lockfile: ./requirements.txt Caused by: 0: Failed to parse lockfile 1: Failed to parse requirements file 2: 0: at line 1, in Tag: pyyaml ^ # Show the RC version works when `--skip-sandbox` is specified ❌ 1 ❯ phylum parse --lockfile-type pip --skip-sandbox ./requirements.txt Generating lockfile for manifest "requirements.txt" using Pip… [ { "name": "PyYAML", "version": "6.0.1", "type": "pypi", "lockfile": "requirements.txt" } ] # Show the RC version fails when `--no-generation` IS specified along with `--skip-sandbox` ❯ phylum parse --lockfile-type pip --skip-sandbox --no-generation ./requirements.txt ❗ Error: could not parse lockfile: ./requirements.txt Caused by: 0: Failed to parse lockfile 1: Failed to parse requirements file 2: 0: at line 1, in Tag: pyyaml ^ ```

Expected Behavior

Python lockifests are recognized as manifests when they are manifests and parsing is NOT attempted when --no-generation is supplied as an option for the parse/analyze commands. Instead, the error code 20 should be returned, along with the same message as when other manifests are attempted to be parsed with the --no-generation option:

❗ Could not parse lockfile: Parsing "./requirements.txt" requires lockfile generation, but it was disabled through the CLI

Python lockifests are recognized as lockfiles when they are lockfiles and parsing is attempted when --no-generation is supplied as an option for the parse/analyze commands.

Additional Context

N/A

[cd-work]

I'd argue this is expected behavior. You're telling the lockfile generator to parse a file and saying that no lockfile generation should be performed. So if the file is a valid lockfile but the contents aren't a valid lockfile, shouldn't we error out?

Like what do you suggest should happen if the manifest's content is actually invalid and it's a lockfile? How are we to detect that it failed because it is a manifest, when we have no way to determine it is one?

[maxrake]

I'd argue this is expected behavior. You're telling the lockfile generator to parse a file and saying that no lockfile generation should be performed. So if the file is a valid lockfile but the contents aren't a valid lockfile, shouldn't we error out?

Having --no-generation specified does not automatically mean the dependency file is a known lockfile. It could still be a manifest. Erroring out is still expected...but with a specific error code (20) and a message that explains what happened instead of a parsing stack trace.

Like what do you suggest should happen if the manifest's content is actually invalid and it's a lockfile? How are we to detect that it failed because it is a manifest, when we have no way to determine it is one?

The current lockfile generation documentation for lockifests states:

Phylum handles these files by first attempting to analyze them as a lockfile. If anything in the file is not fully specified, this will fail, and Phylum will silence the error and proceed to lockfile generation.

The suggestion here for what should happen is a modification to say:

Phylum handles these files by first attempting to analyze them as a lockfile. If anything in the file is not fully specified, this will fail, and Phylum will silence the error and proceed to lockfile generation unless lockfile generation is suppressed (e.g., via --no-generation option). In that case, error code 20 will be returned along with a message explaining how the lockifest could not be parsed as a lockfile and requires lockfile generation to parse as a manifest, but lockfile generation was disabled through the CLI.

That is likely too wordy to use in the final documentation update, but the intent is hopefully more clear.

[maxrake]

Just to clarify, your complaint here is simply about the error code, correct?

Yes...the error code and the inscrutable output.

[cd-work]

That is likely too wordy to use in the final documentation update, but the intent is hopefully more clear.

Yeah at that point reading the documentation just becomes a chore.

I also do not believe we should return error code 20 in this case. It doesn't make any sense when we cannot determine the cause of the failure.