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

Lychee not detecting markdown files #68

Closed
vipulgupta2048 opened this issue Dec 24, 2021 · 13 comments · Fixed by #73
Closed

Lychee not detecting markdown files #68

vipulgupta2048 opened this issue Dec 24, 2021 · 13 comments · Fixed by #73
Labels
bug Something isn't working

Comments

@vipulgupta2048
Copy link
Contributor

I am running Lychee as a GitHub action on https://github.com/balena-io/docs/, Lchee's GitHub action got updated v1.2.0 which lead to the pattern **/*.md failing.

For some reason, this in the file https://github.com/balena-io/docs/blob/master/.github/workflows/link-checker.yml

with:
           args: >
             --config ./lychee.toml
             --github-token ${{ secrets.GITHUB_TOKEN }}
             **/*.md

translates to just finding 2 markdown files in the repository when the action is running. Refer: https://github.com/balena-io/docs/runs/4626369590?check_suite_focus=true#step:5:19

lychee --format markdown --output /tmp/lychee/out.md --config ./lychee.toml --github-token *** pages/404.md shared/icon.md

I am trying to debug if there is something on our end that changed or is it Lychee's action update that caused this. I ran Lychee locally to confirm the pattern lychee --no-progress --config lychee.toml **/*.md and that worked just fine. Do let me know if you need any more information to help debug this further.

@vipulgupta2048
Copy link
Contributor Author

This issue might have been better off in https://github.com/lycheeverse/lychee-action/issues instead. I thought I was opening it there, can admins transfer this to the other repository.

@mre mre transferred this issue from lycheeverse/lychee Jan 5, 2022
@mre
Copy link
Member

mre commented Jan 5, 2022

lychee **/*.md uses the glob expansion from your shell while lychee "**/*.md" uses lychee's internal glob expansion (with the glob crate). So if you use a different shell locally than gets used in Github Actions, the result might be different. Try quoting the glob for consistent behavior.
Unfortunately there's not much we can do other than making sure that all examples show quoted glob expressions. We'll probably also switch the default action to checking all markdown and html files by default thanks to the new lychee ., which walks over the directory tree recursively and in parallel so it is much faster than glob. This is available in the latest lychee master, but not in the action yet.

So in summary, can you try quoting the glob until we publish a new version of the action?

@vipulgupta2048
Copy link
Contributor Author

vipulgupta2048 commented Jan 6, 2022

You got it, I created a PR following your suggestion: balena-io/docs#2168
Here's the error I see.
Screenshot from 2022-01-07 00-59-43

@vipulgupta2048
Copy link
Contributor Author

Ran and approved your PR run: https://github.com/balena-io/docs/pull/2169/files

@mre
Copy link
Member

mre commented Jan 7, 2022

I'm afraid that will probably not work. :/
There's a syntax error in the if condition, but more importantly I couldn't make it work yesterday in #69. You can check at the commits to see what I tried.

  • unquoted -> your current issue
  • '**/*.md' -> recursive wildcards must form a single path component
  • './**/*.md' -> this is what the error above tries to tell us I guess, but it now finds no files

I'm a bit out of ideas for the moment as to what this could be.
From what I heard, globset behaves slightly differently, so perhaps we should try that next.

@mre
Copy link
Member

mre commented Jan 7, 2022

See also: rust-lang/glob#83

We wanted to move to globset at some point anyway, because then we can support multiple glob patterns and exclusions: lycheeverse/lychee#284

@mre mre added the bug Something isn't working label Feb 4, 2022
mre added a commit that referenced this issue Feb 6, 2022
Glob patterns were not properly evaluated for three reasons:

1. Quotes around globs were not preserved. As a result, unquoted wildcards were evaluated by bash instead of lychee
2. `**` patterns handled by the [glob](https://github.com/rust-lang-nursery/glob) crate need to be prefixed with a separator, e.g. `./`. See code [here](https://github.com/rust-lang-nursery/glob/blob/master/src/lib.rs#L536) and [here](https://github.com/rust-lang-nursery/glob/blob/337d417ee872dc04e8e6faff4b7379141933df59/src/lib.rs#L583-L596). We should probably switch to [globset](https://github.com/BurntSushi/globset) at some point, which doesn't have that limitation.
3. The lychee command itself needs to be executed with `eval` to make it find links. Otherwise it interprets the input argument (`${ARGS[@]}`) as a string and tries to find links within that string. (String input is supported by lychee). We want to interpret it as individual (whitespace-separated) arguments however.

**Usage inside pipelines: Surround glob patterns by single (!) quotes and prefix them with `./`.**

```yaml
- name: Link Checker
  uses: lycheeverse/[email protected]
  with:
    # Check all Markdown and HTML files
    args: --verbose --no-progress './**/*.md' './**/*.html'
```

Fixes #68
@mre
Copy link
Member

mre commented Feb 6, 2022

@vipulgupta2048, #73 should fix it. It was a combination of issues. To make sure this doesn't happen anymore in the future, I've added some integration tests.
I'll go ahead and merge it so you can give it a shot by using lycheeverse/lychee-action@master.

@mre mre closed this as completed in #73 Feb 6, 2022
mre added a commit that referenced this issue Feb 6, 2022
Glob patterns were not properly evaluated for three reasons:

1. Quotes around globs were not preserved. As a result, unquoted wildcards were evaluated by bash instead of lychee
2. `**` patterns handled by the [glob](https://github.com/rust-lang-nursery/glob) crate need to be prefixed with a separator, e.g. `./`. See code [here](https://github.com/rust-lang-nursery/glob/blob/master/src/lib.rs#L536) and [here](https://github.com/rust-lang-nursery/glob/blob/337d417ee872dc04e8e6faff4b7379141933df59/src/lib.rs#L583-L596). We should probably switch to [globset](https://github.com/BurntSushi/globset) at some point, which doesn't have that limitation.
3. The lychee command itself needs to be executed with `eval` to make it find links. Otherwise it interprets the input argument (`${ARGS[@]}`) as a string and tries to find links within that string. (String input is supported by lychee). We want to interpret it as individual (whitespace-separated) arguments however.

**Usage inside pipelines: Surround glob patterns by single (!) quotes and prefix them with `./`.**

```yaml
- name: Link Checker
  uses: lycheeverse/[email protected]
  with:
    # Check all Markdown and HTML files
    args: --verbose --no-progress './**/*.md' './**/*.html'
```

Fixes #68
@mre
Copy link
Member

mre commented Feb 6, 2022

Merged to master now. Please let me know if you encounter any additional issues. If not, I'll release a new version from master soon.

@vipulgupta2048
Copy link
Contributor Author

Hey @mre Thanks for your effort on this.
master version works now, check out the run here: https://github.com/balena-io/docs/runs/5093030921?check_suite_focus=true
Except for a few thread panic errors that I see in the logs, I think the glob paths work after putting quotes around them.

@mre
Copy link
Member

mre commented Feb 7, 2022

Yeah the thread panic is because of seanmonstar/reqwest#1399. Not much we can do about it other than waiting or maintaining a fork of reqwest with a patch until this gets merged. 😕 I'm actually considering to create a temporary fork just for this.

@mre
Copy link
Member

mre commented Feb 7, 2022

Apart from that, thanks for the feedback. 😄

@vipulgupta2048
Copy link
Contributor Author

Right on right on, looking forward to the new release for the action.

@mre
Copy link
Member

mre commented Feb 8, 2022

Alright, new release is out. 😄 https://github.com/lycheeverse/lychee-action/releases/tag/v1.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants