/snapcraft

Export important project variables #757

Merged
merged 12 commits into from Aug 29, 2016

Conversation

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

sergiusens commented Aug 25, 2016

This generally exports:

  • SNAPCRAFT_PROJECT_NAME
  • SNAPCRAFT_PROJECT_VERSION
  • SNAPCRAFT_PROJECT_STAGE

and for each part:

  • SNAPCRAFT_PART_INSTALL

LP: #1588336

Signed-off-by: Sergio Schvezov sergio.schvezov@ubuntu.com
@sergiusens
Export important project variables 
This generally exports:

- `SNAPCRAFT_PROJECT_NAME`
- `SNAPCRAFT_PROJECT_VERSION`
- `SNAPCRAFT_PROJECT_STAGE`

and for each part:

- `SNAPCRAFT_PART_INSTALL`

LP: #1588336

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>

All checks have passed

4 successful checks
@snappy-m-o
autopkgtest integration — Success
Details
@snappy-m-o
autopkgtest snaps — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.0008%) to 97.545%
Details
34a5fd3

@sergiusens sergiusens referenced this pull request Aug 25, 2016

Merged

Ant properties, build target, and destination directory options #752

@sergiusens
Collaborator

sergiusens commented Aug 25, 2016

hm, to actually support all these in fixed yaml entries like source we'd need to move _expand_env to the loader
integration_tests/test_environment.py
+ self.assertThat(test_name, FileContains('test-environment'))
+ self.assertThat(test_version, FileContains('0.1'))
+ self.assertThat(test_stage, FileContains(stage_dir))
+ self.assertThat(test_part_install, FileContains(part_install_dir))
@elopio

elopio Aug 25, 2016

Member

Just a little style nit, I would put the assertions right after each of the vars.

abs_project_dir = os.path.join(os.path.abspath('.'), project_dir)
stage_dir = os.path.join(abs_project_dir, 'stage')

test_name = os.path.join(stage_dir, 'test_name')
self.assertThat(test_name, FileContains('test-environment'))

test_version = os.path.join(stage_dir, 'test_version')
self.assertThat(test_version, FileContains('0.1'))

test_stage = os.path.join(stage_dir, 'test_stage')
self.assertThat(test_stage, FileContains(stage_dir))

part_install_dir = os.path.join(abs_project_dir, 'parts', 'env', 'install')
test_part_install = os.path.join(stage_dir, 'test_part_install')
self.assertThat(test_part_install, FileContains(part_install_dir))
@elopio
Member

elopio commented Aug 25, 2016

I like it! Are you doing that move to the loader?
@sergiusens
Collaborator

sergiusens commented Aug 25, 2016

El jueves, 25 de agosto de 2016 12h'12:57 ART, Leo Arias
notifications@github.com escribió:

I like it! Are you doing that move to the loader?

Yes, moving the _expand_env logic to the loader is a must now.

Enviado con Dekko desde mi dispositivo Ubuntu

sergiusens added some commits Aug 25, 2016

@sergiusens
replace it all
b9731a9
@sergiusens
make qa happy
a562fd6
@sergiusens
move a test to a more suited home

All checks have failed

4 failing checks
@snappy-m-o
autopkgtest integration
Details
@snappy-m-o
autopkgtest snaps
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
coverage/coveralls — Coverage decreased (-0.004%) to 97.54%
Details
1a3b460
integration_tests/test_environment.py
+ self.assertThat(test_stage, FileContains(stage_dir))
+
+ test_part_install = os.path.join(stage_dir, 'test_part_install')
+ self.assertThat(test_part_install, FileContains(part_install_dir))
@elopio

elopio Aug 25, 2016

Member

this is incredibly nicer, don't you think? Like ten thousand times better, you owe me a beer!

integration_tests/snaps/snapcraft-key-values/snapcraft.yaml
+parts:
+ env:
+ plugin: dump
+ source: $SNAPCRAFT_PROJECT_NAME-$SNAPCRAFT_PROJECT_VERSION
@elopio

elopio Aug 25, 2016

Member

I don't understand this one.
The source will be translated to test-environment-0.1, so are you missing that subdirectory?

@kyrofa

kyrofa Aug 25, 2016

Member

Indeed, I assume this will fail.

@sergiusens

sergiusens Aug 26, 2016

Collaborator

indeed it has, commiting empty directories in github is a chore ;-)

@kyrofa

kyrofa Aug 26, 2016

Member

Yeah, .keep.

snapcraft/internal/project_loader.py
+ ]
+
+ def _expand_env(self, snapcraft_yaml):
+ skip_keys = ['name', 'version']
@elopio

elopio Aug 25, 2016

Member

a better name: environment_keys.
Then the comment below is not required.

@kyrofa

kyrofa Aug 25, 2016 •
Edited 1 time

Member

I'm still trying to figure out why these are skipped in the first place. The code might be a little cleaner if you just went ahead and checked them. Would it hurt anything?

@sergiusens

sergiusens Aug 26, 2016

Collaborator

@kyrofa I wanted to avoid something weird like setting

name: $SNAPCRAFT_PROJECT_NAME
@kyrofa

kyrofa Aug 26, 2016

Member

Yeah, I figured. Man is that ever the corner of cases 😛 . But valid nonetheless.

@elopio
Member

elopio commented Aug 25, 2016

I still like it! Even better that it now factored the stage dir. 👍
integration_tests/snaps/snapcraft-key-values/snapcraft.yaml
+summary: test the SNAPCRAFT_PROJECT_.* variables in key values
+description: |
+ Tests the snapcraft exported variables in values for key arguments
+ This will fail to pull if the resulting values are not replaced.
@kyrofa

kyrofa Aug 25, 2016 •
Edited 1 time

Member

Maybe this should include "or if the source directory doesn't exist" 😉 .

integration_tests/test_environment.py
+ self.assertThat(test_part_install, FileContains(part_install_dir))
+
+ def test_project_environment_within_snapcraft(self):
+ """Replace the SNAPCRAFT_PROJECT_.* ocurrences in snapcraft.yaml
@kyrofa

kyrofa Aug 25, 2016

Member

s/ocurrences/occurrences/

@kyrofa
Member

kyrofa commented Aug 25, 2016 •
Edited 1 time

This is lovely @sergiusens, I'm a big fan! There might be an improvement to be had in _expand_env(), but I'm 👍 either way.

And of course, the broken tests. Coverage dropped, too.

sergiusens added some commits Aug 26, 2016

@sergiusens
add a file to get a dir
38b8aa9
@sergiusens
typo

All checks have failed

3 failing checks
@snappy-m-o
autopkgtest integration
Details
@snappy-m-o
autopkgtest snaps
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
9bf6425
@sergiusens
env-keys

Some checks were not successful

3 failing and 1 successful checks
@snappy-m-o
autopkgtest integration
Details
@snappy-m-o
autopkgtest snaps
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
coverage/coveralls — Coverage increased (+0.003%) to 97.547%
Details
874229d
@sergiusens
static

Some checks were not successful

1 failing and 3 successful checks
@snappy-m-o
autopkgtest snaps
Details
@snappy-m-o
autopkgtest integration — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.003%) to 97.547%
Details
6ea9c79
@sergiusens
Collaborator

sergiusens commented Aug 26, 2016

retest this please
@sergiusens
Merge branch 'master' into feature/1588336/export-vars

Some checks were not successful

1 failing and 3 successful checks
@snappy-m-o
autopkgtest snaps
Details
@snappy-m-o
autopkgtest integration — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.003%) to 97.819%
Details
2fcbecd
@elopio
Member

elopio commented Aug 26, 2016

this still looks good to me.

sergiusens added some commits Aug 26, 2016

@sergiusens
make copies of mutables
0673311
@sergiusens
Merge branch 'feature/1588336/export-vars' of github.com:sergiusens/s… 
…napcraft into feature/1588336/export-vars

Some checks were not successful

3 failing and 1 successful checks
@snappy-m-o
autopkgtest integration
Details
@snappy-m-o
autopkgtest snaps
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
coverage/coveralls — Coverage increased (+0.004%) to 97.82%
Details
c6fb3f6
@sergiusens
fix units

All checks have passed

4 successful checks
@snappy-m-o
autopkgtest integration — Success
Details
@snappy-m-o
autopkgtest snaps — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.004%) to 97.82%
Details
8c91d3b
@sergiusens
Collaborator

sergiusens commented Aug 27, 2016

�[0;32mSnapped mosquitto_0.1_amd64.snap�[0mWARNING:snaps_tests.testbed:Running from /tmp
oh mosquitto...
retest this please just in case

@sergiusens sergiusens merged commit dc0b681 into snapcore:master Aug 29, 2016
4 checks passed

4 checks passed

autopkgtest integration Success
Details
autopkgtest snaps Success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.004%) to 97.82%
Details

@sergiusens sergiusens deleted the sergiusens:feature/1588336/export-vars branch Aug 29, 2016

evandandrea pushed a commit to evandandrea/snapcraft that referenced this pull request Aug 29, 2016

Drop ant-dist-property in favour of #757.
3cbf888

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

@sergiusens
Export important project variables (#757) 
* Export important project variables

This generally exports:

- `SNAPCRAFT_PROJECT_NAME`
- `SNAPCRAFT_PROJECT_VERSION`
- `SNAPCRAFT_STAGE`

and for each part:

- `SNAPCRAFT_PART_INSTALL`

LP: #1588336

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>
e4504dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment