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

redis url and windows exe #162

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sebbean
Copy link

@sebbean sebbean commented Feb 17, 2025

No description provided.

@matvp91
Copy link
Collaborator

matvp91 commented Feb 17, 2025

Hi @sebbean,

Would you mind making two issues for this? If I understand it correctly, you'd like to have the following:

  • Support for Windows (ffmpeg, ffprobe) binaries.
  • Support for REDIS_URL, which is currently split between a host and a port.

I'll take up additional questions in those issues.

Regarding feature nr. 2, I added a patch to support this (in favor of HOST/PORT), https://github.com/superstreamerapp/superstreamer/pull/164/files, I noticed in your PR you missed a few places. Happy to have that one merged already if it fixes your case.

@sebbean sebbean marked this pull request as draft February 18, 2025 13:03
@sebbean
Copy link
Author

sebbean commented Feb 18, 2025

@matvp91 yes! i'll button this up with some more clarity early next week!

i ended up being able to run in windows no problem, just had to build the packages in wsl first - ha

also the .exe to the end of the bins downloaded

@sebbean
Copy link
Author

sebbean commented Feb 18, 2025

@matvp91 this is the only error stopping build on windows AFAIK:

@superstreamer/api build: [757ms] bundle 1329 modules
@superstreamer/api build: cp: illegal option -- r
@superstreamer/api build: Exited with code 1

something in the build scripts is using cp which is not avail on windows ps

@matvp91
Copy link
Collaborator

matvp91 commented Feb 18, 2025

Isn't cp available on Windows Powershell?

The build step isn't really meant for people to run, it's designed for CI/CD to build the Docker images. As a user, you'd run the Docker images. As a developer, you'd run bun run dev which does not require the build step. I believe spinning up the dev environment on Windows does work (and if it doesn't, that's a bug I'd have to fix) but I can't test myself. Would you like to check this for me?

Very unlikely we'd have a build step compliant on Windows as CI/CD does not support this.

Would you be willing to drop this PR in favor for another one that contains only the changes needed for downloading and running the .exe binaries on Windows? I'd be happy to assist you.

I like the change you made where REDIS_HOST and REDIS_PORT (multiple) are dropped in favor of REDIS_URI (single) and abstracted it in a separate PR where I put you as Co-Authored. 088468b is merged to main, thank you for bringing this one up.

@sebbean
Copy link
Author

sebbean commented Feb 18, 2025

yes i can help with the exe PR - i'll modify or reopen a new PR for that step

re: cp on powershell
https://stackoverflow.com/a/21734429/27800494

re: build vs dev,

I saw an issue and may have noticed one saying you have to run build before the dev command works properly

i'm new to bun so wasnt sure if that was the case

@matvp91
Copy link
Collaborator

matvp91 commented Feb 18, 2025

I saw an issue and may have noticed one saying you have to run build before the dev command works properly

That'll no longer be necessary, getting the dev env up and running should be as simple as bun run dev. The project has changed a fair bit in structure past few weeks and dev scripts shall take care of package building for you.

I assume you've checked out the main branch, thus the command above should be enough to get you started.

If you can use PowerShell instead, which should come pre-installed in modern Windows systems, you can use cp and some other Unix commands directly in it.

In your StackOverflow link, doesn't this answer imply you're able to cp with PowerShell?

Pretty cool to see there's no need for major changes to get developers with Windows into the project, thank you for the time invested!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants