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.

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

Vale sometimes fails to run, complaining of non-existent files #3764

Closed
sarayourfriend opened this issue Feb 7, 2024 · 4 comments · Fixed by #3871
Closed

Vale sometimes fails to run, complaining of non-existent files #3764

sarayourfriend opened this issue Feb 7, 2024 · 4 comments · Fixed by #3871
Assignees
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: mgmt Related to repo management and automations

Comments

@sarayourfriend
Copy link
Contributor

Description

@stacimc and I have both run into this issue in different ways. Here is Staci's example output of the error:

docker run --rm -v $PWD/..:/src:rw,Z --workdir=/src openverse-vale:local . --glob='!*{_build,_static,venv,.venv,.nuxt,.pnpm-store,node_modules,test-results,storebook-static,.ruff-cache,projects/proposals,changelogs/api,changelogs/frontend,changelogs/catalog,changelogs/ingestion_server}*'
stat venv/bin/python: no such file or directory
error: Recipe `run` failed on line 64 with exit code 2

I had the same error mode, but the file was src/static/node_modules/sharp (I'd created a node_modules folder in frontend/src/static when working on #3724). I was able to resolve the issue for me locally by deleting that directory, and didn't think anything much of it because I'd done something rather unusual, and I didn't think it would affect normal usage. However, it appears to indeed affect normal usage, as now Staci is running into the same problem.

Something tricky about it, however, is that we can't easily tell where the file causing the issue is, because Vale's message cuts off the first directory!

Reproduction

Not sure how to reproduce at the moment. I tried recreating the issue I had locally, but wasn't able to reproduce it.

Additional context

Marked high because it interrupts local development workflows.

@sarayourfriend sarayourfriend added 🟧 priority: high Stalls work on the project or its dependents 🛠 goal: fix Bug fix 🤖 aspect: dx Concerns developers' experience with the codebase 🧱 stack: mgmt Related to repo management and automations labels Feb 7, 2024
@zackkrida
Copy link
Member

I'll attempt to look at this this week. I want to learn more about how Vale works anyway; perhaps in my stumbling around I'll trigger this issue 😆

@sarayourfriend
Copy link
Contributor Author

I hope you can discover the cause. I have a feeling it will be a bug in Vale we can open an issue for as well as something to do with docker bind mounts and symlinks to files outside the bind mount scope (which Docker cannot traverse).

If you haven't tried it yet, I'd suggest creating a symlink to something outside the Openverse directory. It might be able to reproduce the issue.

@zackkrida
Copy link
Member

zackkrida commented Mar 1, 2024

I didn't get to spend quite as much time on this as I would have liked, but I can confirm that it's easily reproducible using an approach similar to @sarayourfriend's initial problem:

# Copy a directory with symlinks into somewhere it doesn't belong 
cp -r node_modules frontend/src/static
# Run vale
bash -c 'just .vale/run'  
# Output
stat frontend/src/static/node_modules/@openverse/eslint-plugin: no such file or directory
error: Recipe `run` failed on line 64 with exit code 2

Next, I installed a Vale 2.30.0 binary locally and converted our justfile, dockerized vale config into a command I could run locally, for easier debugging (I also had to vale sync to install proselint):

vale --output=line --minAlertLevel=suggestion --config=./.vale.ini --ext=.md --glob='!*{_build,_static,venv,.venv,.nuxt,.pnpm-store,node_modules,test-results,storebook-static,.ruff-cache,projects/proposals,changelogs/api,changelogs/frontend,changelogs/catalog,changelogs/ingestion_server}*' .

From here, I was able to reproduce the error with the local binary:

❯ vale --output=line --minAlertLevel=suggestion --config=./.vale/.vale.ini --ext=.md --glob='!*{_build,_static,venv,.venv,.nuxt,.pnpm-store,node_modules,test-results,storebook-static,.ruff-cache,projects/proposals,changelogs/api,changelogs/frontend,changelogs/catalog,changelogs/ingestion_server}*' .
stat frontend/src/static/node_modules/@openverse/eslint-plugin: no such file or directory

I confirmed that this is a broken symlink:

❯ ls -la frontend/src/static/node_modules/@openverse
total 56
drwxr-xr-x  2 zack zack  4096 Mar  1 16:53 .
drwxr-xr-x 68 zack zack 49152 Mar  1 16:53 ..
lrwxrwxrwx  1 zack zack    28 Mar  1 16:53 eslint-plugin -> ../../packages/eslint-plugin

Next, I tested Vale 3.x to see if the problem would magically go away. It didn't. 😄

Finally, I tried to isolate the problem file with different glob patterns. I got the same original error every time regardless of what pattern I used or if a --glob flag was present at all!

Sadly, I had to stop here! I want to look into the actual code more, starting here:

https://github.com/errata-ai/vale/blob/3d92f00671a22bd9616f3b4242ca8f5a793c29b9/internal/glob/glob.go

I also want to experiment with not using --glob and instead pre producing our own list of files to pass as the input to vale. That should give us more control.

@zackkrida
Copy link
Member

Success! Taking an approach of building a list of files for the input of Vale, rather than using their globbing, worked well! For example:

openverse on  refactor/combine-related-media [$!?] via ⬢ v18.19.1 via 🐍 v3.11.2 on ☁️  (us-east-2) 
❯ vale --output=line --minAlertLevel=suggestion --config=./.vale/.vale.ini $(find . -type f | grep -vE '_build|_static|venv|.venv|.nuxt|.pnpm-store|node_modules|test-results|storebook-static|.ruff-cache|projects/proposals|changelogs/api|changelogs/frontend|changelogs/catalog|changelogs/ingestion_server')

frontend/README.md:3:1:Openverse.TermCasing:Incorrect casing. Use 'Openverse' instead of 'OpenVerse'. # <-- This is an edit I made so I could confirm there's a Vale output

So essentially, if we update our Justfile with a step to build a file list to pass to Vale (unless the user has explicitly passed files, in which case we should use those), we should be able to prevent this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: mgmt Related to repo management and automations
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants