/snapcraft

Add usr/bin to the shebang replace path #238

Merged
merged 1 commit into from Jan 17, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@sergiusens @kyrofa @elopio
@sergiusens
Collaborator

sergiusens commented Jan 15, 2016

LP: #1534812
@kyrofa
Member

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
Member

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
Collaborator

sergiusens commented Jan 16, 2016

I am thinking of rebasing with #236
@kyrofa
Member

kyrofa commented Jan 16, 2016

Does #236 look good? Feel free to merge and go ahead and rebase.
examples/ros/snapcraft.yaml
+ stage:
+ - -$dev
+ snap:
+ - -$dev
@elopio

elopio Jan 16, 2016

Member

This makes ROS an awesome example. Cool.

examples_tests/tests.py
@@ -178,6 +178,9 @@ class TestSnapcraftExamples(testscenarios.TestWithScenarios):
'dir': 'ros',
'name': 'ros-example',
'version': '1.0',
+ 'external_tests_commands': [
+ ("sed -n '/env/p;1q' snap/usr/bin/rosversion",
+ b'#!/usr/bin/env python\n',)],
@elopio

elopio Jan 16, 2016

Member

how clever this guy who invented the external test commands ;)
I would explain in a comment what that sed statement is doing there.

@sergiusens

sergiusens Jan 16, 2016

Collaborator

done

snapcraft/repo.py
@@ -298,6 +308,14 @@ def _fix_filemode(path):
os.chmod(path, mode & 0o1777)
+def _fix_shebangs(path):
@elopio

elopio Jan 16, 2016

Member

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

@sergiusens

sergiusens Jan 16, 2016

Collaborator

done

@elopio
Member

elopio commented Jan 16, 2016

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

sergiusens commented Jan 16, 2016

@elopio I fixed the code to take into account your comments 😄
@sergiusens
Add bin paths to the to the shebang replace paths in repo 
LP: #1534812

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.3%) to 90.605%
Details
cd9c111
@sergiusens
Collaborator

sergiusens commented Jan 16, 2016

@kyrofa done

sergiusens added a commit that referenced this pull request Jan 17, 2016

@sergiusens
Merge pull request #238 from sergiusens/bugfix/1534812/more-shebangs 
Add usr/bin to the shebang replace path
59a23a5

@sergiusens sergiusens merged commit 59a23a5 into snapcore:master Jan 17, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 90.605%
Details

@sergiusens sergiusens deleted the sergiusens:bugfix/1534812/more-shebangs branch Jan 17, 2016

smoser pushed a commit to smoser/snapcraft that referenced this pull request Sep 14, 2016

Corrected extension version selection and stdout/stderr redirection 
Corrects extension version selection (issue #238).
Corrects hanlding of stdout / stderr (issue #260).

Signed-off-by: Brendan Dixon <brendand@microsoft.com>
2c44b88

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

@sergiusens
Merge pull request #238 from sergiusens/bugfix/1534812/more-shebangs 
Add usr/bin to the shebang replace path
5187df1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment