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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snap #2778

Merged
merged 37 commits into from Dec 2, 2019

Conversation

@casperdcl
Copy link
Member

casperdcl commented Nov 12, 2019

  • remove stage: check # TODO: remove before merge from .travis.yml
  • add snapcraft.yml
  • add deploy.provider: snap in .travis.yml
    • postpone to after store approval. use GitHub releases in the meantime
  • ensure version info PKG = "snap"
  • ensure basic build
  • ensure advanced build (all optional features & dependencies excluding tests)
  • port over completion (https://github.com/iterative/dvc/blob/master/scripts/completion)
  • remove "update/newer version available" message as snaps auto-update themselves
  • add icon
  • fix travis gssapi build
  • minimise build (maybe don't use python plugin?)
  • test connectivity and features
  • add SNAP_TOKEN to travis env var config
  • release on snap store (edge channel)
  • decide whether travis should auto-release to stable rather than just edge
    • decide on multiple release channels, e.g.:
      • on: tags -> stable or <tag>/stable or both (snap install dvc --classic)
      • branch: master -> edge (snap install dvc --classic --edge)
      • branch: <other> -> edge/<other>
  • update readme with installation instructions
    • decide on badge
  • fixes #2771
  • basic dev ref: https://docs.travis-ci.com/user/deployment/snaps/

Get it from the Snap Store

though personally I prefer

snapcraft

which I may have possibly contributed as a badge simple-icons/simple-icons#1215 馃槢

Before final approval, though, you can still test (after installing snap) by downloading from https://dashboard.snapcraft.io/snaps/dvc/revisions/6/download and doing snap install --dangerous --classic <filename.snap>

$ snap install --dangerous --classic <filename.snap>
$ dvc version
DVC version: 0.68.1+47e1b4
Python version: 3.6.8
Platform: Linux-4.15.0-58-generic-x86_64-with-Ubuntu-16.04-xenial
Binary: False
Package: snap
Cache: reflink - False, hardlink - True, symlink - True
casperdcl added 4 commits Nov 11, 2019
.travis.yml Outdated Show resolved Hide resolved
@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 12, 2019

And it's 2:15am... I should probably stop working on this for now.

Get it from the Snap Store

@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 12, 2019

@casperdcl Get some sleep from the... Nap Store... * sorry * 馃檪

@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 12, 2019

@casperdcl https://snapcraft.io/dvc Looks great! 馃敟

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 12, 2019

fyi

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 12, 2019

also I need a list of your ubuntu one email addresses (you should sign up for a dev account via https://dashboard.snapcraft.io/) so I can add admin access to the snap for your accounts.

@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 12, 2019

@casperdcl I actually have an ubuntu one account, tied to my gmail email (see my profile).

casperdcl added 2 commits Nov 12, 2019
@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 12, 2019

@casperdcl Do I understand correctly, that this can automatically deploy to snap on every merged PR (for edge) and also for every tag (for stable)? If so, that is amazing.

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 12, 2019

@efiop

The format for channels is [<track>/]<risk>[/<branch>]

So yes, we could do all of:

  • on: tags: <tag>/stable or latest/stable or both
  • branch: master: latest/edge
  • branch: <other>: latest/edge/<other>

The last one would make it pretty easy for people to test out feature-branches...

@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 12, 2019

Sounds great! No need to worry about feature branches, as we don't have those :) At least for now. master(for edge) and tags(for stable) would be perfect for now. If I understand edge purpose correctly, of course 馃檪

casperdcl added 2 commits Nov 12, 2019
.travis.yml Show resolved Hide resolved
casperdcl added 4 commits Nov 12, 2019
snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/snapcraft.yaml Show resolved Hide resolved
README.rst Outdated

.. code-block:: bash
snap install dvc

This comment has been minimized.

Copy link
@efiop

efiop Nov 29, 2019

Member

So no additional flags needed for now? I'm completely lost with that classic approval. Is this even going to work without that approval?

This comment has been minimized.

Copy link
@casperdcl

casperdcl Nov 30, 2019

Author Member

added some explicit commands which users would need to run to grant permission (classic would grant everything)

This comment has been minimized.

Copy link
@casperdcl

casperdcl Nov 30, 2019

Author Member

I haven't tested all functionality so there's likely a few permissions missing...

This comment has been minimized.

Copy link
@casperdcl

casperdcl Nov 30, 2019

Author Member

I just did all of https://dvc.org/doc/tutorials/versioning and only got stuck at the end with dvc run since with strict confinement there's no way to access host system binaries (including e.g. python). Not something we can fix without --classic...

This comment has been minimized.

Copy link
@efiop

efiop Nov 30, 2019

Member

Isn't there a way to explicitly tell snap to install with "classic" permissions even if it is dangerous?

This comment has been minimized.

Copy link
@casperdcl

casperdcl Nov 30, 2019

Author Member

the snap won't be auto-downloadable (snap install --dangerous --classic dvc won't work). Users would have to download the snap file from the pending approval section (might even need logging in?) or from github releases and then snap install --dangerous --classic <dvc_VER.snap>

This comment has been minimized.

Copy link
@casperdcl

casperdcl Nov 30, 2019

Author Member

Maybe the latter is the only feasible option for now.

casperdcl added 5 commits Nov 30, 2019
@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 30, 2019

argh I'm close to giving up. Debugged all the plugs, tried confinement: devmode, and they still don't allow it. 馃挗

In any case confinement: classic will definitely be required for all functionality to work.

@efiop I'd recommend removing installation instructions from the readme for now pending classic approval.

@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 30, 2019

@casperdcl Who "they"? Don't see any reaction from snap guys on the forum.

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Nov 30, 2019

The automatic review bot runs checks and it turns out that even when using strict or devmode the system-files permission (required for reading /etc/hosts, /etc/host.conf, /etc/resolv.conf) still requires approval. And of course dvc run wouldn't work without classic. Nor do repos in certain types of external filesystem mounts.

casperdcl added 3 commits Nov 30, 2019
README.rst Show resolved Hide resolved
@efiop

This comment has been minimized.

Copy link
Member

efiop commented Nov 30, 2019

@casperdcl Looks good, just one more comment about README ^ .

casperdcl added 2 commits Dec 2, 2019
- didn't work
- almost completely reverts c94acdf
@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Dec 2, 2019

@efiop done I think

@efiop efiop merged commit d42a04e into iterative:master Dec 2, 2019
4 checks passed
4 checks passed
DeepSource: Python Analysis Passed: No blocking issues detected.
Details
Travis CI - Pull Request Build Passed
Details
codeclimate All good!
Details
restyled No differences
Details
@casperdcl casperdcl deleted the casperdcl:snap branch Dec 2, 2019
@shcheklein

This comment has been minimized.

Copy link
Member

shcheklein commented Dec 6, 2019

@casperdcl should we review and update our docs for linux? or are we waiting for an official approval?

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Dec 6, 2019

Updated the readme in this PR but waiting for approval (#2760 (comment)) before going further I think

@shcheklein

This comment has been minimized.

Copy link
Member

shcheklein commented Dec 6, 2019

@casperdcl got it! Snap looks cool 馃槑 thanks for the effort ... when it's done may be it will be enough to get rid of pyinstaller indeed.

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Dec 6, 2019

Hah I'm not sure that'll die any time soon. In the words of Joshua Bloch, "APIs are forever," and I think installation methods fall under the same umbrella, unfortunately.

@shcheklein

This comment has been minimized.

Copy link
Member

shcheklein commented Dec 6, 2019

:)

@casperdcl you are breaking my last hope :(

@casperdcl

This comment has been minimized.

Copy link
Member Author

casperdcl commented Dec 6, 2019

Hey, maaaybe in a year I'll consider dropping support for py2.6 in the packages I maintain. It's not hopeless; just hopeslow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.