Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add new plugin: rust #579
Conversation
snappy-m-o
commented
Jun 17, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Jun 17, 2016
|
Can one of the admins verify this patch? |
mmstick
commented
Jun 17, 2016
|
Would very much like to see this as I have a hard time packaging my Rust applications without it. |
WebDrake
reviewed
Jun 18, 2016
| + | ||
| + properties = schema['properties'] | ||
| + self.assertTrue('rust-channel' in properties, | ||
| + 'Expected "rust-verion" to be included in properties') |
WebDrake
Jun 18, 2016
Looks like there are some typos here and in the self.assertTrue statements below: verion where I presume version is usually meant ... ?
Although in the specific line above, it looks like it should be Expected "rust-channel"
tsimonq2
reviewed
Jun 18, 2016
| + mock.call([plugin._cargo, 'install', | ||
| + '-j{}'.format(plugin.project.parallel_build_count), | ||
| + '--root', plugin.installdir]) | ||
| + ]) |
tsimonq2
Jun 18, 2016
Contributor
Nice work so far!
Coveralls points out that you are missing some tests here. In line 84 of that Coveralls page for example, shows that there is no test for an invalid Rust channel.
I don't have commit access and Coveralls could be totally wrong, but I thought I should point it out just in case Coveralls is right.
sergiusens
reviewed
Jun 18, 2016
| @@ -0,0 +1,3 @@ | ||
| +fn main() { | ||
| + println!("There is rust on snaps!"); |
sergiusens
reviewed
Jun 18, 2016
| +For more information check the 'plugins' topic for the former and the | ||
| +'sources' topic for the latter. | ||
| + | ||
| +Additionally, this plugin uses the following plugin-specific keyword: |
sergiusens
reviewed
Jun 18, 2016
| + "--root", self.installdir]) | ||
| + | ||
| + def env(self, root): | ||
| + env = ["RUSTC=%s" % self._rustc, |
sergiusens
Jun 18, 2016
Collaborator
This env should be used for runtime environment variables for when the snap app runs
You should probably look at how it is done in the gulp plugin where it does a os.environ.copy, sets these and calls run with this env
sergiusens
reviewed
Jun 18, 2016
| + | ||
| + def pull(self): | ||
| + super().pull() | ||
| + self._fetch_rust() |
sergiusens
Jun 18, 2016
Collaborator
This is missing a clean_pull to wipe out the rust specific dir for the part made by the plugin (look at the nodejs plugin for inspiration)
|
Hi there, thank you very much for this. I have little to no idea how rust works, but seems you have an integration test and everything so kudos! I added a few comments here and there, but this looks really good already! |
|
Also, no need to push your branch to see if things pass, I bet turnaround time would be faster for the smaller tests by running |
mariogrip
added some commits
Jun 18, 2016
|
ok to test |
snappy-m-o
commented
Jun 20, 2016
|
Can one of the admins verify this patch? |
sergiusens
reviewed
Jun 21, 2016
| +from snapcraft.plugins import rust | ||
| + | ||
| + | ||
| +class MakePluginTestCase(tests.TestCase): |
|
A rebase or update/merge branch with master and updating that trivial comment would be good. Going to say ok to test just in case something else is missing |
mariogrip
added some commits
Jun 24, 2016
|
@sergiusens ready to test |
snappy-m-o
commented
Jun 24, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Jun 24, 2016
|
Can one of the admins verify this patch? |
|
ok to test |
|
@mariogrip oh, this has conflicts now :-/ If it is list plugins I guess that needs to be mocked! (our problem, not yours) |
|
@sergiusens I fixed the conflict :) |
|
El sábado, 25 de junio de 2016 10h'24:28 ART, Marius Gripsgard
Tests still fail though
Enviado con Dekko desde mi dispositivo Ubuntu |
|
Thanks a lot for the great contribution! This is missing tests for revision and channel, and for the clean pull. Let us know here or on IRC if you need a hand with this. pura vida. |
|
I'm trying to use your plugin to build https://github.com/rolandshoemaker/theca Here's my yaml: https://paste.ubuntu.com/18598588/ |
snappy-m-o
commented
Jul 6, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Jul 6, 2016
|
Can one of the admins verify this patch? |
boghison
commented
Jul 12, 2016
•
|
@elopio You need nightly rust to build that, not stable AFAIU (just add |
snappy-m-o
commented
Jul 12, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Jul 12, 2016
|
Can one of the admins verify this patch? |
|
Can the failing tests be retried? There was some interest in this: https://twitter.com/boghison/status/753144022445723648?cn=cmVwbHk%3D&refsrc=email |
|
First the conflicts need to be resolved. |
sergiusens
added
the
code-conflict
label
Jul 13, 2016
boghison
commented
Jul 16, 2016
•
|
Hi! I resolved the conflicts and squashed the commits in 427e3a6, but I have test fails in |
|
@boghison I'm not seeing any test failures in |
boghison
commented
Jul 22, 2016
|
mariogrip
added some commits
Jul 22, 2016
|
Sorry for the lack of updates on this, i have been a bit busy. Ok, so now at least travis build returns success. But autopkgtest return error:
Anyone know why? |
|
@mariogrip I think this when it's trying to download the rustup.sh script. @elopio are the autopkgtests run behind a firewall that might be blocking access to https://static.rust-lang.org/rustup.sh. FWIW, the test passes for me locally. |
|
Yes they are behind a firewall. Let me request the exception. |
|
retest this please |
mariogrip commentedJun 17, 2016
This adds plugin for rust (https://www.rust-lang.org/)