Skip to content

Conversation

sergiusens
Copy link
Collaborator

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

This exposes the prime directory inside the build provider onto the host
and messages the user to run "snap try" on that directory.

LP: #1805212

Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
Copy link
Collaborator Author

Hah, I will have to split out those lxd snapcraft try tests as they fail require snapcraft to come from the store which is unfortunate as we still have no injection for lxd

@codecov-io
Copy link

codecov-io commented Apr 5, 2019

Codecov Report

Merging #2524 into master will decrease coverage by 0.08%.
The diff coverage is 67.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2524      +/-   ##
==========================================
- Coverage   89.17%   89.08%   -0.09%     
==========================================
  Files         201      201              
  Lines       13615    13659      +44     
  Branches     2057     2065       +8     
==========================================
+ Hits        12141    12168      +27     
- Misses       1039     1052      +13     
- Partials      435      439       +4
Impacted Files Coverage Δ
snapcraft/cli/lifecycle.py 87.87% <100%> (+2.87%) ⬆️
snapcraft/internal/build_providers/_lxd/_lxd.py 23.66% <16.66%> (-0.54%) ⬇️
snapcraft/internal/lifecycle/_clean.py 75% <68.18%> (-3.77%) ⬇️
.../internal/build_providers/_multipass/_multipass.py 83.06% <71.42%> (-0.7%) ⬇️
...apcraft/internal/build_providers/_base_provider.py 83.97% <83.33%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4900f73...9a9e5be. Read the comment docs.

@@ -272,7 +298,8 @@ def pack(directory, output, **kwargs):
required=False,
help="Forces snapcraft to use LXD for this clean command.",
)
def clean(parts, use_lxd):
@click.option("--unprime", is_flag=True, required=False, cls=HiddenOption)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note, newer click versions added hidden=true so we can use that when we upgrade click.

Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

The implementation of this feature is not obvious and addresses a number of small pitfalls found when exposing the provider-built prime back to the outer environment, including unpriming/repriming (but I see no painless alternative to this). Overall the implementation is functional and probably as clean as it can be given all the details it must deal with. Good work!

@sergiusens
Copy link
Collaborator Author

Any time we need to deal with state inside the container/VM it becomes a matter of re-execution and knowing where you are standing, so yeah, you are correct, this has not been the easiest :-)

@sergiusens sergiusens merged commit ec579ea into canonical:master Apr 8, 2019
@sergiusens sergiusens deleted the snapcraft-try branch April 8, 2019 20:34
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.

3 participants