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

dont assume .exe is the only executable extension on windows #1

Merged
merged 1 commit into from
May 23, 2017
Merged

dont assume .exe is the only executable extension on windows #1

merged 1 commit into from
May 23, 2017

Conversation

bbatha
Copy link
Contributor

@bbatha bbatha commented May 23, 2017

This should probably be a search over PATH_EXT in the future, but that may still have some edge cases. For instance, emscripten generates .js files which are intended to be executed and the user would likely not have in her PATH_EXT.

This should probably be a search over PATH_EXT in the future, but that may still have some edge cases. For instance, emscripten generates .js files which are intended to be executed and the user would likely not have in her PATH_EXT.
@koute
Copy link
Owner

koute commented May 23, 2017

Does this actually fix anything? I'm not against merging this; I'd just like to understand what impact it would have. Did you change this because it fixes an actual problem you were facing?

(On a side note: this function is kinda broken by design, and I'd love to remove it completely, but that would require that rust-lang/cargo#3670 gets fixed; alternatively we could always launch cargo with the --message-format json parameter and format the user-facing messages ourselves, but that seems like a huge amount of work for something as basic as printing out the filename of the file which cargo just generated.)

@bbatha
Copy link
Contributor Author

bbatha commented May 23, 2017

Does this actually fix anything? I'm not against merging this; I'd just like to understand what impact it would have. Did you change this because it fixes an actual problem you were facing?

Ya cargo-web on windows doesn't detect .js files as executable so test and start don't pickup the built artifacts.

@koute
Copy link
Owner

koute commented May 23, 2017

Hmm... well, right now I don't entirely understand why this would work, but as I currently have no way of verifying it I'm going to take your word on it and merge it. (Setting up appveyor for cargo-web is already on my very long TODO list.)

Thanks!

@koute koute merged commit db007da into koute:master May 23, 2017
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