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

Add usr/bin to the shebang replace path #238

Merged

Conversation

sergiusens
Copy link
Collaborator

LP: #1534812

@kyrofa
Copy link
Contributor

kyrofa commented Jan 15, 2016

👍 from me, though we may want to do this to all .debs in the future. Just waiting for the tools.

@kyrofa
Copy link
Contributor

kyrofa commented Jan 15, 2016

Ah right, the downside to doing it this way: It also needs to be done to the roscore plugin or the file contents differ.

@sergiusens sergiusens force-pushed the bugfix/1534812/more-shebangs branch 4 times, most recently from 5e6bb49 to b602f5d Compare January 16, 2016 16:08
@sergiusens
Copy link
Collaborator Author

I am thinking of rebasing with #236

@kyrofa
Copy link
Contributor

kyrofa commented Jan 16, 2016

Does #236 look good? Feel free to merge and go ahead and rebase.

stage:
- -$dev
snap:
- -$dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes ROS an awesome example. Cool.

@@ -298,6 +308,14 @@ def _fix_filemode(path):
os.chmod(path, mode & 0o1777)


def _fix_shebangs(path):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't hurt to comment why the shebangs have to be fixed. Just like on fix_symlinks above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@come-maiz
Copy link
Contributor

+1 from me. I'm just missing a couple of comments.

@sergiusens
Copy link
Collaborator Author

@ElOpio I fixed the code to take into account your comments 😄

@sergiusens
Copy link
Collaborator Author

@kyrofa done

sergiusens added a commit that referenced this pull request Jan 17, 2016
@sergiusens sergiusens merged commit 59a23a5 into canonical:master Jan 17, 2016
@sergiusens sergiusens deleted the bugfix/1534812/more-shebangs branch January 17, 2016 11:28
smoser pushed a commit to smoser/snapcraft that referenced this pull request Sep 14, 2016
Corrects extension version selection (issue canonical#238).
Corrects hanlding of stdout / stderr (issue canonical#260).

Signed-off-by: Brendan Dixon <brendand@microsoft.com>
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
…shebangs

Add usr/bin to the shebang replace path
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