Snappy: Add support for snapcraft builds #1898
Conversation
ci/snapcraft.sh
Outdated
| # as rustup.$PROXY. If we have an alias which is not a supported proxy name | ||
| # then rustup might get sad. | ||
|
|
||
| PROXIES="cargo cargo-clippy cargo-fmt cargo-miri clippy-driver rls rustc rustdoc rustfmt" |
There was a problem hiding this comment.
We may want to use bash feature - array instead.
There was a problem hiding this comment.
I'm not a shell person. I'm happy if you can tell me what to do there, otherwise this can stay until you want to fix it later
There was a problem hiding this comment.
Basic uses of bash array if you want to try: https://devhints.io/bash#arrays
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
805fb5a
to
0589416
Compare
|
@lzutao Thank you for the comprehensive review. I believe I've tackled everything you hilighted (except the one thing I don't feel confident in) -- could you recheck please? |
ci/snapcraft.sh
Outdated
| # as rustup.$PROXY. If we have an alias which is not a supported proxy name | ||
| # then rustup might get sad. | ||
|
|
||
| PROXIES="cargo cargo-clippy cargo-fmt cargo-miri clippy-driver rls rustc rustdoc rustfmt" |
There was a problem hiding this comment.
Basic uses of bash array if you want to try: https://devhints.io/bash#arrays
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
0589416
to
593b35c
Compare
|
r=me with or with out using bash array, unless others disagree. |
|
Thank you @lzutao We won't be merging this until I've spoken to the release team about snaps, but it's good to know you're now satisfied with my awful shell code :-D |
|
☔ The latest upstream changes (presumably #1916) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Now that we're a bit further on in our releases, and Windows builds are no longer always failing; I'll give this some more thought. |
|
I've started a discussion with Snap people about github actions for this kind of thing. Depending on how that goes, I'll either rework this PR, or close it and open a fresh one for continued discussion around |
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
593b35c
to
37a0191
Compare
kinnison
force-pushed
the
kinnison/snapcraft
branch
2 times, most recently
from
e20c125
to
96a35fd
Compare
|
I now need to wait for an appropriate publish action to be written and we can proceed further. |
There was a problem hiding this comment.
I've left a few drive-by comments after the discussion on the Snapcraft forum. I've been working on an action to publish snaps here:
https://github.com/jhenstridge/snapcraft-publish-action
I haven't quite locked down the API yet, but it is functional. As the action depends on a secret (the Snap Store credentials), I'm not sure how easy it will be to test within a pull request.
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
96a35fd
to
d6891d2
Compare
|
@jhenstridge Thank you for your input and review -- I'll look at the publish action you've provided and I can probably test it by adding the secret to my own repo fork and trying it there to ensure it behaves OK. I'll try and do that soon. |
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
d6891d2
to
5c17fc9
Compare
|
@jhenstridge I think that ought to do the trick, obviously I'm tied to a particular commit of your repo for now. I tested it over at my own fork and it seemed to do the right thing, so now I need to wait for you to finalise a v1 of the publish action I guess, and then decide on the final comments you made on the snapcraft yaml file. |
kinnison
force-pushed
the
kinnison/snapcraft
branch
2 times, most recently
from
655362c
to
d2f3f1f
Compare
|
I believe everything is right now, and I've added the token to the repo's secrets ready for post-merge use. If noone objects, I'll merge this soon and we can see how that goes. |
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
d2f3f1f
to
de0df60
Compare
|
Is the shell script shellchecked automatically? |
|
@rbtcollins Yes, shellcheck is run over all our shell scripts as part of the CI. |
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
de0df60
to
c64074d
Compare
To begin the work of supporting snaps of rustup, we need to first ensure we can build the snaps in the first place. Signed-off-by: Daniel Silverstone <[email protected]>
Signed-off-by: Daniel Silverstone <[email protected]>
kinnison
force-pushed
the
kinnison/snapcraft
branch
from
c64074d
to
de2b7a0
Compare
|
@rbtcollins I believe your question and change were resolved. I've updated to use the now official snapcore actions. I think we're likely good to merge assuming the CI remains green. Opinions? |
|
I have decided to go ahead and merge this because we need to work through any problems long before we make a release, and we can only do that once it's trying to push to snapstore etc. |
This commit enables building snaps for the various platforms supported by snapcraft.io
Currently the snap is under my personal page, and despite building it here, we'll keep that until after all-hands where I'll have a chat with t-release people about what we might do for an official snap. So far I've had no joy looking at official methods of distributing rustup in other channels such as flatpak.