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

[Bug]: actionhero generate does nothing on windows #2275

Closed
1 task done
YamCrack opened this issue Aug 29, 2022 · 11 comments · Fixed by #2278
Closed
1 task done

[Bug]: actionhero generate does nothing on windows #2275

YamCrack opened this issue Aug 29, 2022 · 11 comments · Fixed by #2278
Labels
bug Something isn't working

Comments

@YamCrack
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

when i run command

npx actionhero generate

nothing happens.

WindowsTerminal_rb21nChhHC

Expected Behavior

New project created

Stack Trace

No response

Environment

OS: Windwos 11, Window 10
Actionhero Verision: 28.1.11
Nodejs: 14.20.0

Additional context

No response

@YamCrack YamCrack added the bug Something isn't working label Aug 29, 2022
@YamCrack
Copy link
Contributor Author

the command works fine on actionhero 28.1.9

@evantahler
Copy link
Member

Hi!

Can you confirm that the directory you tried actionhero generate in is empty? Also, run npx actionhero --version to confirm which version you have locally.

I have no access to a windows machine to do debug this... so let me know if you are able to work on this issue yourself!

@YamCrack
Copy link
Contributor Author

yes i have tried with empty directory and after npm i -s actionhero, none of them works, i can try to fix it.

@YamCrack
Copy link
Contributor Author

I found the problem is the update of glob from 7.20 to 8.0.1

Since glob 7.2.1 backslash “\” is not supported and it only uses slash “/”, everytime something like this is used:

glob.sync(path.join(configPath, “**“, “**/*(*.js|*.ts)“))

path.join adds a backslash. Therefore, the path is no longer valid. If you agree, we can change it to:

glob.sync(path.join(configPath, “**“, “**/*(*.js|*.ts)“), { windowsPathsNoEscape: true })

or join the paths manually.

Please let me know your thoughts.

@evantahler
Copy link
Member

That's a shame that windowsPathsNoEscape isn't automatically enabled if a windows environment is detected. Ideally, that would be a feature added to glob itself. If that isn't possible, I guess we could wrap it in a utility...

Can you try working with the glob maintainers first to create a "windows compat" mode?

@YamCrack
Copy link
Contributor Author

YamCrack commented Sep 1, 2022

There are issues related to this problem and they are refusing to change it. They recommend doing something like this:

path.join(__dirname, '*.txt').split(path.sep).join("/")

sorry for my bad english

@evantahler
Copy link
Member

Can you link the conversation in the Glob repo please? I wonder if they make good points that we should follow as well.

@YamCrack
Copy link
Contributor Author

YamCrack commented Sep 1, 2022

sure, there is:

isaacs/node-glob#480
isaacs/node-glob#468
isaacs/node-glob#419

-- we can write a utility function that check por path.sep, and change the \ to /.
like this:
image

@evantahler
Copy link
Member

evantahler commented Sep 2, 2022

This conversation does a good job of explaining why replacing the slashes probably isn't a good idea - isaacs/node-glob#468

I think the path forward is to replace all uses in the project of glob with safeGlob() which internally attempts to detect which OS the project is running on. If windows is detected, then we enable windowsPathsNoEscape as we call into glob.

function safeGlobSync(match: string, ...args) { 
  const isWindows: process.platform === "win32" || process.platform === "win64";
  return glob.sync(match, {...args, windowsPathsNoEscape: isWindows})
} 

@YamCrack
Copy link
Contributor Author

YamCrack commented Sep 2, 2022

i did a PR, let me know your thoughts

398241e

@YamCrack
Copy link
Contributor Author

YamCrack commented Sep 6, 2022

Ia have been pull new changes:

  • test for safeGlobSync
  • pettify for lint faling files,

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