Repository navigation
Cannot build on Windows due to Bash usage #4720
Description
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
SDK Version
master
Steps to Reproduce
yarn && yarn build
Expected Result
Builds
Actual Result
Fails on bash scripts/buildBundles.sh
Activity
lobsterkatie commentedon Mar 17, 2022
@timfish - Can you say more about what actually goes wrong? What about the script is it unhappy with?
timfish commentedon 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 commentedon Mar 18, 2022
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 commentedon Mar 18, 2022
Yup, you need a WSL (now) or Cygwin, MinGW or msys (old days) 🥲
Lms24 commentedon 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)
timfish commentedon Mar 18, 2022
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.
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 commentedon 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 commentedon Mar 18, 2022
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-nodeto our repo this morning, so we don't even have to resort to raw JS!timfish commentedon 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 commentedon Mar 21, 2022
@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
timfish commentedon Mar 21, 2022
Yes I'll take this on. I'll leave the above mentioned files for now!
timfish commentedon Mar 28, 2022
Both
postbuild.shandbuildbundles.shlook quite simple so if @lobsterkatie or @Lms24 are otherwise busy let me know and I can tackle them!lobsterkatie commentedon Mar 29, 2022
I'm actually in the middle of revamping
buildBundlesfor other reasons, but you could def hop onpostbuild. 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 commentedon Mar 29, 2022
@timfish I was planning on working on
postbuild.shby the end of this week (around thursday, friday) while making some build structure changes. If you get to it earlier, feel free to go aheadref(dev): Run linter on scripts (#4834)