Open j9367 opened 2 years ago
Assign issue to self (@jackm9367)
Looks like this is more than just re-writing install.js Due to the way the installation re-builds argon, and the structure of the core module, node-pre-gyp errors out with a bug on Windows (SS attached). Recommendation for the moment is that this issue is closed and left dormant to just support macOS/Linux for the time being. Alternative option is to try and find issue within node-pre-gyp and create issue, however this could lead to further problems. If @gao-sun and @demonzoo are happy to close as not planned, or just add to longterm roadmap (probably not in Q3 sprint) then can close issue without PR. Issue with node-pre-gyp
thank you @jackm9367 - I didn't test developing on Windows, and it looks like the issue is straightforward. we can give it a try to see the effort. this issue can keep open, no worries.
Would you like to try it in git bash env?
Will give it a go now
I'd be guessing though that if Logto only 'supports' windows through git bash
, then it doesn't really count as windows support?
Was able to download and extract the file using the one-liner in git bash
, but a few issues came up:
couldn't run npx node-pre-gyp
as npx-pre-gyp wasn't found. After installing the node-pre-gyp
package (npm i -g node-pre-gyp
) I was able to run the npx node-pre-gyp rebuild -C .
command, but came up with an error stating Error: Cannot find module node-addon-api'
node-addon-api
, installed successfully via npm
, but then the node-pre-gyp
command still returned same error. Tried outside of git bash in powershell, returned same error. Had a look at this issue and ran npm install --global --production windows-build-tools
in administrative powershell, which timed out so didn't install. Additionally, windows-build-tools
is already included in node 16.x so shouldn't need to install. Auto-start did not work after install, giving error that 'NODE_ENV' is not recognized as an internal or external command,
. I'm guessing that it would be something to do with the fact that it is trying to set an environment variable after running the cd
command, but this also shouldn't be an issue.
At this stage it's just an issue of rebuilding the argon2 package then we should be good. Will keep investigating best way forward.
Thank you, Jack. I'll also take a look at it this weekend.
Will give it a go now I'd be guessing though that if Logto only 'supports' windows through
git bash
, then it doesn't really count as windows support?
I agree! But I also think that git bash
might just be working as a baseline for windows users for the time being, until we work out a full compatible windows script.
Hi Jack, how are you doing lately? It seems it's been awhile since we talked about this last time. And due to the inactivity of more than 3 weeks, we are going to take it back from you and see if there's anyone else who is also interested in.
And btw, we are now promoting a "Logto bounty hunter" event, in which you can complete bounties and claim rewards (real money 💰💰💰). And I think this ticket can be put on the bounty board as well. More details about this event can be found from here.
Let me know if you want to claim it back and I can assign it back to you. Cheers!
Hi 👋
I would like to be assigned to this issue if possible please 😄
Hey @charIeszhao and @Olyno I've been off sick for the last while so would love it if you could take this one @Olyno! Sorry for not messaging you @charIeszhao, fell off my radar a bit.....
No problem. @jackm9367 Sorry to hear that, but I hope you are OK now. Feel free to let us know if you would like to contribute something. You are always welcome here
quick update: thanks to @Olyno we're going to launch a cross-platform CLI soon. but I cloud not figure out the permission issue that happened in GitHub Actions Windows, you can see this workflow runs down from #4640 - fought with Windows for the whole day but no luck.
I decided to move on as I'm a noob to Windows stuff meanwhile it is in CI which is hard to debug. I'll test on a real Windows laptop after launch tough. It'll be great if any of you guys would like to test the new CLI and help with the CI issue afterwards. cheers.
guys, i'm wondering with the new CLI can Logto properly download and work in Windows?
As long as the code is a JS code or a .exe
executable, then yes it should work correctly on Windows.
Guys, I just tried npm init @logto
on windows, but I ended up only creating a package.json
file...
It actually makes sense. The npm init
command (or yarn create
or pnpm create
depending on the package manager) works only if the name
of the package starts with create-
.
For example, create-logto
will make the package compatible with the command, but having a package named @logto
will not work, because this one doesn't start with the create-
prefix.
Yes, but the same command npm init @logto
works on macOS and Linux... So are you suggesting that we should rename the package name to create-logto
?
From what I see, it is mostly a bug on Mac and Linux. The package managers should not find the package in question according to their description:
With Yarn and Npm, they specify in their docs that the package name must start with the prefix "create-".
What I would suggest on my side is to rename the package to be conventional. This could fix the problem.
Thanks, @Olyno. That makes sense to me. @gao-sun Any thoughts?
Actually we do have a "@logto/create" package otherwise the command won't work in macOS and Linux. @charIeszhao are you using an old version of npm?
lol, again?? Let me check it real quick...
Nope, it's 8.5.5
The "@" is for scoped package which follows another convention "@org/create".
Actually I registered "create-logto" but didn't published anything yet (just a package.json). Since I'd like to publish it automatically instead of manually creating another package with the same content of "@logto/create".
Nevermind. As Gao said, it is possible to use the organization as a prefix for the "npm init" command. However, this would make the package exclusive to npm, and other package managers would be excluded (Yarn, Pnpm, Deno and potentially Bun).
Even if I understand that creating a package with another name is annoying, this seems to me to be the best thing to do (and therefore deprecated "@logto/create").
You can take the example of Redwood to see how they manage their packages.
thanks @Olyno, I'll take a look
@Olyno
npm create @logto
and pnpm create @logto
are working finecoerceCreatePackageName()
if interested), and it will always be create
for yarn create @logto
. thus there's no way to use it. but yarn 1 is kind of archived. so i think it's ok to leave as it is.for the traditional create-logto
approach, rather than keeping it in sync of @logto/create
, i think we can just print a error message and lead users to use @logto/create
instead since all our packages are scoped.
how does it sound?
The main problem is that the CLI does not work on Window. Creating a package under the organization (@logto/create
) does not seem to fix this problem, quite the opposite.
I understand your concern about creating a package that doesn't contain the organization in the name, but I still think that this is the best way to fix the problem on Window, and make the package compatible under any manager package.
@Olyno thanks for the feedback and sorry for the delayed response. since i don't have a Windows dev environment, @charIeszhao would you like to have a try on this?
What problem did you meet?
The one-liner script outlined in README.md does not function on Windows due to the script utilising curl, printf, and other Linux/Bash commands. When run, the script immediately errors and halts.
Describe what you'd like Logto to have
A separate install script for Windows, including a separate Windows-oriented one liner that downloads and runs a wnidows-installer.js file in node, allowing for a one-line windows install.