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

Fix shell script generation in build.c #2372

Closed
wants to merge 1 commit into from
Closed

Fix shell script generation in build.c #2372

wants to merge 1 commit into from

Conversation

GladOSkar
Copy link

(On my system) the "run" build commands didn't work (the opened term said the run script was not executable). For some reason the code here only made the run script executable on apple platforms. I removed this limitation to have the file chmod'ed on all unixoid platforms.

Works on my machine.

I'm very new to patches and PRs and such, lmk if i'm doing this wrong :)

Cheers

[On my system] the "run" build commands didn't work (the opened term said the run script was not executable). For some reason the code here only made the run script executable on apple platforms. I removed this limitation to have the file chmod'ed on all unixoid platforms.
@elextr
Copy link
Member

elextr commented Oct 18, 2019

Strange that you need it on non-apple, the shell script is run with /bin/sh -c <scriptname> which doesn't care if its executable. Perhaps open an issue with the problem first.

@GladOSkar
Copy link
Author

Interesting, my sh says otherwise:

oskar@torres ~ % /bin/sh -c /tmp/geany_run_script_HVQX9Z.sh
/bin/sh: /tmp/geany_run_script_HVQX9Z.sh: Permission denied

@GladOSkar
Copy link
Author

GladOSkar commented Oct 19, 2019

Thinking about it it makes sense - We're passing a command, not a script, sh ist not interpreting the contents of the script here, but just executing the single command passed: /tmp/geany_run_script_HVQX9Z.sh. That command is for sh to spawn a child process to execute that script via a syscall which is then blocked because the script is not executable.

@GladOSkar
Copy link
Author

Created issue at #2374

@GladOSkar
Copy link
Author

GladOSkar commented Oct 20, 2019

After the issue at #2374 got cleared up as a faulty terminal command, should i leave this open or close it?

I think this change wouldn't hurt and having to add /bin/sh to the terminal command is not very intuitive.

@techee @elextr ?

@techee
Copy link
Member

techee commented Oct 20, 2019

I would personally do the opposite - remove the ifdef code completely as it's not needed. I just did it in #2363.

Even though the default isn't very intuitive, running the executable directly from tmp doesn't work on all distributions so it's not something we should rely on.

@elextr
Copy link
Member

elextr commented Oct 21, 2019

Agree with @techee

@elextr elextr closed this Oct 21, 2019
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.

None yet

3 participants