Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Directories detected as lockfiles #1177

Open
kylewillmon opened this issue Aug 8, 2023 · 1 comment
Open

Directories detected as lockfiles #1177

kylewillmon opened this issue Aug 8, 2023 · 1 comment
Assignees
@cd-work
Labels
bug Something isn't working low priority Should be handled as time permits

Comments

@kylewillmon
Copy link
Contributor

kylewillmon commented Aug 8, 2023 •
edited
Loading

Lockfile detection believes that directories might be lockfiles.

How To Reproduce

> mkdir requirements.txt

> ls
requirements.txt

> phylum parse
Generating lockfile for manifest "./requirements.txt" using Pip…
❗ Error: Lockfile generation failed! For details, see: https://docs.phylum.io/docs/lockfile-generation

Caused by:
    package manager exited with error code 1:

    ERROR: Could not open requirements file: [Errno 21] Is a directory: '/path/to/requirements.txt'

This error message isn't very helpful....

> mkdir Cargo.lock

> phylum parse
❗ Error: Is a directory (os error 21)
@kylewillmon kylewillmon added bug Something isn't working low priority Should be handled as time permits labels Aug 8, 2023
cd-work added a commit that referenced this issue Aug 17, 2023
@cd-work
Ensure lockfiles are files
a56ff06 
This patch introduces a metadata check for lockfile and manifest paths
to ensure that they're files instead of directories.

Closes #1177.
cd-work added a commit that referenced this issue Aug 17, 2023
@cd-work
Ensure lockfiles are files
c2800b8 
This patch introduces a metadata check for lockfile and manifest paths
to ensure that they're files instead of directories.

Closes #1177.
@cd-work cd-work mentioned this issue Aug 17, 2023
Closed
@cd-work cd-work self-assigned this Aug 17, 2023
@cd-work
Copy link
Contributor

cd-work commented Aug 18, 2023

#1202 as a solution was discarded, since even checking for !is_directory still causes syscalls to lookup metadata for every lockfile, even if it doesn't cause failure for the GitHub App. Since this has never caused any issues in practice, there's no reason to slow down the GitHub App.

kylewillmon added a commit that referenced this issue Aug 18, 2023
@kylewillmon
Improve lockfile parsing errors
84168d6 
As shown in #1177, some lockfile parsing errors result in poor error
messages because the lockfile path is not always known.

This patch adds the lockfile path as context on the error to ensure that
it will be present in the error message.

**Before**
```
> phylum parse
❗ Error: Is a directory (os error 21)
```

**After**
```
> phylum parse
❗ Error: could not parse lockfile: ./Cargo.lock

Caused by:
    Is a directory (os error 21)
```
@kylewillmon kylewillmon mentioned this issue Aug 18, 2023
Merged
kylewillmon added a commit that referenced this issue Aug 18, 2023
@kylewillmon
Improve lockfile parsing errors (#1204)
3eb4630 
As shown in #1177, some lockfile parsing errors result in poor error
messages because the lockfile path is not always known.

This patch adds the lockfile path as context on the error to ensure that
it will be present in the error message.

**Before**
```
> phylum parse
❗ Error: Is a directory (os error 21)
```

**After**
```
> phylum parse
❗ Error: could not parse lockfile: ./Cargo.lock

Caused by:
    Is a directory (os error 21)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cd-work cd-work

Labels
bug Something isn't working low priority Should be handled as time permits
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure lockfiles are files phylum-dev/cli
2 participants
@kylewillmon @cd-work