-
Notifications
You must be signed in to change notification settings - Fork 29
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
Do not rely on launcher.sh being executable #184
Conversation
This PR is untested so don't merge it quite yet. |
👍 Arguably, we should do the same for |
Agreed @zarvox . I thought I did that a while ago; if not, then it's a PR or local patch of mine that's lost to the ages. I want some |
Hmmm, I'd kinda like to revert #206 and merge this, since it seems to be a potential blocker for Windows users getting up and running, and there's an alternative approach that achieves the same end state (with one less file read, even!) that I mentioned on that ticket. |
I'm fine with that if it's what you want to do -- #206 was basically a hack to get around #205, the point of which was being able to automate the change you suggest. Would be nice to be able to be a bit more flexible with what you can do with a stack in general. afaik there's no way to do "raw" stacks either, which you can also kinda pull off with Go (and will be more feasible as https://gihtub.com/zenhack/go.sandstorm gets more mature), and not having sandstorm-http-bridge in the spk shaves off 2-3 MiB. Possible alternative: On windows, could just add a mount option that makes the executable bit be the default: https://stackoverflow.com/questions/35807568/vagrant-synced-folder-permissions/35821148#35821148 |
If I'm understanding the bug correctly, we should hit the same problem trying to run a native executable when building on a windows box -- if stuff is non-exec, running /opt/app/app is going to fail. And there's no shell you can pass an ELF to. So perhaps this PR isn't general enough? |
Once you're editing the |
Right, but if /opt/app is a shared folder on a windows host, will doing chmod +x actually work? I thought the whole reason for this patch was that the executable bit wasn't doing what you want on windows hosts. |
Perhaps not. In that case, I guess the alternate approach is having |
That should work. However: do you have any objections to the mount option solution I proposed above? It seems like it would make a lot of things simpler. (We also really ought to test this...) |
Context: pgrm/swagger-editor-sandstorm#4