Skip to content

Cannot build on Windows due to Bash usage #4720

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

Closed
3 tasks done
timfish opened this issue Mar 15, 2022 · 14 comments
Closed
3 tasks done

Cannot build on Windows due to Bash usage #4720

timfish opened this issue Mar 15, 2022 · 14 comments
Labels
Package: browser Issues related to the Sentry Browser SDK

Comments

@timfish
Copy link
Collaborator

timfish commented Mar 15, 2022

Is there an existing issue for this?

SDK Version

master

Steps to Reproduce

yarn && yarn build

Expected Result

Builds

Actual Result

Fails on bash scripts/buildBundles.sh

@lobsterkatie
Copy link
Member

@timfish - Can you say more about what actually goes wrong? What about the script is it unhappy with?

@timfish
Copy link
Collaborator Author

timfish commented Mar 17, 2022

Windows does not have bash so the script completely fails to run.

I suppose WSL might work but this was never a requirement before and it's a huge dependency.

@lobsterkatie
Copy link
Member

Windows does not have bash so the script completely fails to run.

Forgive my ignorance, but really? I feel like shell scripts are all over the place in software development. Can folks who use windows machines truly not run any of them (without WSL, at least, which is sounds like isn't a part of your normal setup or an everyday thing)? What do you do instead? There are other bash scripts in the repo. How do you run them?

@kamilogorek
Copy link
Contributor

Yup, you need a WSL (now) or Cygwin, MinGW or msys (old days) 🥲

@Lms24
Copy link
Member

Lms24 commented Mar 18, 2022

This also applies to scripts/postbuild.sh (currently) when building @sentry/browser
(although this could be easily replaced by a Node script, I suppose)

@Lms24 Lms24 added the Package: browser Issues related to the Sentry Browser SDK label Mar 18, 2022
@timfish
Copy link
Collaborator Author

timfish commented Mar 18, 2022

I feel like shell scripts are all over the place in software development.

Shell scripts may be common in some areas of software development but they are rare in the node.js ecosystem.

JavaScript projects stick to package.json scripts and node.js scripts to retain cross platform compatibility and to reduce barriers to outside contribution.

For things like Relay where there are few outside contributions, Windows support doesn't really matter but for a repository like this there should be a policy over whether to stick to solely node.js or not.

There are other bash scripts in the repo. How do you run them?

When issues pop up in the build or unit tests I submit PR's to fix them! Currently, I just let the integration tests run in CI.

@timfish
Copy link
Collaborator Author

timfish commented Mar 18, 2022

Oh, and I'm happy to do a PR to remove shell script usage if that's direction we want to go!

@lobsterkatie
Copy link
Member

Wow, well - TIL. (I think you're literally the only person I know who works on a windows machine, LOL.)

We chatted about it at our standup, and we're good to move to only node scripting going forward (probably everywhere, but at least for build for sure). And thanks for the offer to help! The bash scripts that @Lms24 and I have introduced over the last week or two it's probably just as easy for us to convert since they're fresh in our minds, but if there are others that need fixing, by all means - we'd be glad for the assistance!

Coincidentally, I actually just merged a PR adding ts-node to our repo this morning, so we don't even have to resort to raw JS!

@timfish
Copy link
Collaborator Author

timfish commented Mar 18, 2022

I've mainly worked on desktop software so need at least one device per platform. I've even got a cheapo laptop that was emergency purchased specifically to tune and test multi-touch touchscreen interactions. I spend the majority of my time and travel with whichever device is newest/fastest and that's currently Windows. I'd guess by the end of the year I'll have a new Mac and that will become the default 😂

Drop me a message here if you've got any scripts you want me to tackle!

@smeubank
Copy link
Member

@timfish so is this something you are comfortable taking on?

only thing to note, is that these are some scripts which have been recently added and @lobsterkatie or @Lms24 would like to change them themselves

  • postbuild.sh
  • buildbundles.sh

@timfish
Copy link
Collaborator Author

timfish commented Mar 21, 2022

Yes I'll take this on. I'll leave the above mentioned files for now!

@timfish
Copy link
Collaborator Author

timfish commented Mar 28, 2022

Both postbuild.sh and buildbundles.sh look quite simple so if @lobsterkatie or @Lms24 are otherwise busy let me know and I can tackle them!

@lobsterkatie
Copy link
Member

I'm actually in the middle of revamping buildBundles for other reasons, but you could def hop on postbuild. Let's just coordinate here before you do, though, because I believe @Lms24 is about to make a few more small changes to it in the next few days.

@Lms24
Copy link
Member

Lms24 commented Mar 29, 2022

@timfish I was planning on working on postbuild.sh by the end of this week (around thursday, friday) while making some build structure changes. If you get to it earlier, feel free to go ahead

lobsterkatie added a commit that referenced this issue Mar 31, 2022
Because SDK developers running Windows [can't run our bash scripts[1], we recently decided to move to using TypeScript instead. To make this easier, this extends the domain of our `eslint` and `prettier` checks to include all `scripts` folders. It also fixes a few linting errors which doing so brought to light.

[1] #4720
@HazAT HazAT closed this as completed Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: browser Issues related to the Sentry Browser SDK
Projects
None yet
Development

No branches or pull requests

6 participants