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

fix: resolve matched files to cwd instead of gitDir before adding #857

Merged
merged 2 commits into from
Apr 30, 2020
Merged

fix: resolve matched files to cwd instead of gitDir before adding #857

merged 2 commits into from
Apr 30, 2020

Conversation

iiroj
Copy link
Member

@iiroj iiroj commented Apr 29, 2020

Because the set of matched files are collected from generated tasks, when using the relative options they are relative to the current cwd, not the git dir which might be different.

@iiroj iiroj requested a review from okonet April 29, 2020 18:29
@iiroj iiroj mentioned this pull request Apr 29, 2020
Closed
@iiroj iiroj force-pushed the fix-relative-add branch from a71a9d9 to 2519dec Compare April 29, 2020 18:57
okonet
okonet previously approved these changes Apr 29, 2020
View reviewed changes
@iiroj
Copy link
Member Author

iiroj commented Apr 29, 2020

Need to normalize the asserted path in tests for the Windows test.

@iiroj
fix: resolve matched files to cwd instead of gitDir before adding
defe045 
Because the set of matched files are collected from generated tasks, when using the `relative` options they are relative to the current cwd, not the git dir which might be different.
@iiroj iiroj force-pushed the fix-relative-add branch from 2519dec to d38b283 Compare April 29, 2020 19:11
@iiroj
Copy link
Member Author

iiroj commented Apr 29, 2020 •
edited
Loading

@okonet I split into two commits because there was technically another fix in there.

EDIT: argh, one more test, one more missing normalize.

@iiroj iiroj requested a review from okonet April 29, 2020 19:12
@iiroj iiroj force-pushed the fix-relative-add branch from d38b283 to dd9240a Compare April 30, 2020 03:51
@iiroj iiroj force-pushed the fix-relative-add branch from dd9240a to ba67f48 Compare April 30, 2020 04:13
@iiroj
Copy link
Member Author

iiroj commented Apr 30, 2020

@okonet it seems the Appveyor test is failing to something not-test-related. Can you re-trigger it manually somehow?

@okonet
Copy link
Collaborator

okonet commented Apr 30, 2020

I just did but it's still failing with the same message:

error Command failed with exit code 3221225477.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Command exited with code -1073741819

There is no information behind this link... Ideas? Can it be Node 14?

@okonet
Copy link
Collaborator

okonet commented Apr 30, 2020

Okay I restarted the whole build and now it seem to work: https://ci.appveyor.com/project/okonet/lint-staged

@okonet
Copy link
Collaborator

okonet commented Apr 30, 2020

@iiroj let me know if that's good to merge and I'll merge it since the check wasn't picked up here but it's all green now.

@iiroj
Copy link
Member Author

iiroj commented Apr 30, 2020

@okonet thanks, please do! A merge commit would be better to preserve both fixes in the changelog.

@okonet okonet merged commit 5883296 into master Apr 30, 2020
@okonet okonet deleted the fix-relative-add branch April 30, 2020 11:29
@github-actions
Copy link
Contributor

🎉 This PR is included in version 10.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ram3shyadav
Copy link

ram3shyadav commented Apr 30, 2020 •
edited
Loading

@okonet @iiroj

still not fixed
for git, file location is Development/WebApps/Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts

but git is trying to add Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts which is relative to package.json or cwd

  lint-staged:git Running git command [ 'diff', '--staged', '--diff-filter=ACMR', '--name-only', '-z' ] +11ms
  lint-staged:run Loaded list of staged files in git:
  lint-staged:run [ 'Development/WebApps/Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts' ] +39ms
  lint-staged:chunkFiles Resolved an argument string length of 97 characters from 1 files +0ms
  lint-staged:chunkFiles Creating 1 chunks for maxArgLength of 131072 +1ms
  lint-staged:gen-tasks Generating linter tasks +0ms
  lint-staged:gen-tasks Generated task: 
  lint-staged:gen-tasks { pattern: '*.{ts,js,html,css,scss,json}',
  lint-staged:gen-tasks   commands: [ 'prettier --write' ],
  lint-staged:gen-tasks   fileList:
  lint-staged:gen-tasks    [ 'Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts' ] } +9ms
  lint-staged:gen-tasks Generated task: 
  lint-staged:gen-tasks { pattern: '*.ts',
  lint-staged:gen-tasks   commands: [ 'tslint --fix' ],
  lint-staged:gen-tasks   fileList:
  lint-staged:gen-tasks    [ 'Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts' ] } +1ms

  lint-staged:git Adding task modifications to index... +774ms
  lint-staged:git Running git command [ 'add',
  '--',
  'Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts' ] +839ms
✖ fatal: pathspec 'Digital/DigitalPlatform/ClientApp/src/app/api/interceptors/api.interceptor.ts' did not match any files```

@iiroj
Copy link
Member Author

iiroj commented Apr 30, 2020 •
edited
Loading

What, I set it to resolve to absolute paths... 🤔 Let me see again.

EDIT: LOL, found the issue. This PR fixed it for the case where maxArgLenth is not defined, but left it unfixed for all else.

@iiroj iiroj mentioned this pull request Apr 30, 2020
Merged
@iiroj
Copy link
Member Author

iiroj commented Apr 30, 2020

@Ramesh-Yadav I opened #858 to hopefully fix it for good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okonet okonet okonet approved these changes

Assignees
No one assigned
Labels
released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iiroj @okonet @ram3shyadav