Skip to content

Conversation

@timfish
Copy link
Contributor

@timfish timfish commented Oct 23, 2022

It appears that path.resolve already caters for path separator differences so you can safely use forwards slashes and everything works.

Closes #1368

@kamilogorek
Copy link
Contributor

kamilogorek commented Oct 24, 2022

Ah, nice cache, you are correct:

On Windows, both the forward slash (/) and backward slash () are accepted as path segment separator

https://nodejs.org/api/path.html#pathsep

Although I'm not sure how your original issue happened, as this code should work just fine on windows, and we never had any reports of it being broken 🤔

@kamilogorek kamilogorek enabled auto-merge (squash) October 24, 2022 08:28
@timfish
Copy link
Contributor Author

timfish commented Oct 24, 2022

Yes, the cli was working perfectly well on Windows even with the dodgy file name.

However @sentry/nextjs does a check to see if the binary exists (ie. has been successfully downloaded) and this would not have worked on Windows because a) it was not checking for .exe b) the executable file was not where it was expected to be!

https://github.com/getsentry/sentry-javascript/blob/4b137b72e256b3773d9a8b500e857ce3ee7208a0/packages/nextjs/src/config/webpack.ts#L459-L473

@kamilogorek kamilogorek merged commit b236c7e into getsentry:master Oct 24, 2022
@timfish timfish deleted the fix/windows-binary-location branch October 25, 2022 14:24
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.

On Windows, the binary is copied to an incorrect path

2 participants