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 a plugin for running simple shell scripts #252
Conversation
kyrofa
reviewed
Jan 22, 2016
| +steps that fit to no other tool and produces a result after them. | ||
| + | ||
| +All the scripts called by this plugin have to accept at least one argument. | ||
| +This first argument defines the directory on which the resulting files should |
kyrofa
reviewed
Jan 22, 2016
| @@ -0,0 +1,88 @@ | ||
| +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- | ||
| +# | ||
| +# Copyright (C) 2015 Canonical Ltd |
|
@LefterisJP thanks for the PR! Please make sure you sign the CLA. |
kyrofa
reviewed
Jan 22, 2016
| + path to the script file to execute | ||
| + - destination-dir: | ||
| + (string) | ||
| + path relative to the install directory where result/s should be placed |
kyrofa
reviewed
Jan 22, 2016
| + 'sh', | ||
| + os.path.join(self.builddir, script_file_name), | ||
| + dst | ||
| + ]) |
kyrofa
Jan 22, 2016
Member
run() accepts an argument to set the working directory-- I suggest using that.
LefterisJP
commented
Jan 22, 2016
|
@LefterisJP ah, that means my checker script needs updating! Thanks for that. Yeah, on your tests you can mock stdout, like this test. |
LefterisJP
commented
Jan 22, 2016
|
@kyrofa All right done, thanks for the link. Nothing shows up when running the tests now. |
|
There was some discussion about this in the mailing list and the conclusion, iirc, was that it promotes a style for packaging that we want to avoid with snapcraft. I can't find the link now, so my memories might be wrong. @sergiusens should remember better. |
LefterisJP
commented
Jan 22, 2016
|
@elopio This plugin is meant as a last resort if none of the other plugins can do what you want at the moment and/or if you want something very specific done. The more advanced snapcraft becomes, especially around the intricacies of providing cross-compiled binaries for different architectures the less useful such a plugin will be. But we are not there yet. And for the here and now I can definitely see use cases for it. I have one in a snap I am working on right now for example. |
sergiusens
reviewed
Jan 23, 2016
| + 'script': { | ||
| + 'type': 'string', | ||
| + }, | ||
| + 'destination-dir': { |
sergiusens
Jan 23, 2016
Collaborator
this feels specific to a certain script and not generic enough.
|
The plugin looks ok, but it does feel very specific to a use case and not generic enough for reuse. I personally believe we lack convention for this to work for everyone so maybe if it is used to build a specific thing, call the plugin thing. Or if you want more autonomy, let me propose this layout:
Where Am I sounding reasonable or am I missing the obvious? |
LefterisJP
commented
Jan 23, 2016
|
Hello @sergiusens . Your proposed layout is what I am currently using in my project. It works and I can keep it like that but I believe that with the minimal convention of having the first argument be the destination directory all kinds of scripts could work. In essense a shell script can do anything if called with sudo. It can be like a generic 'way out' if you don't want to build your own custom plugin and still do your tasks cleanly with snapcraft. Let me give three examples.
There can be many more examples, since a script's nature is to be generic. In the context of snapcraft, as long as we have a destination directory of where the script should output its results while snapping the part, I think it is a generic enough plugin. If you are afraid it's not generic enough there are 3 additions I can see:
In conclusion, I understand that probably the best way to go would be to have a wget plugin, an xgo plugin, and generally a plugin for each of the tools that one would be tempted to turn to a script for. I agree with that idea but since we are not there yet, I think it's fair to allow users of snapcraft have a generic script solution for when no plugins exist, or when the current plugins don't have enough options, or need more customization. |
|
Hi, sorry for taking so long to take a look at your reply. Here's my stance: Problem 1 can be solved with multiple parts and 2 is in a PR now. I'm closing this for now; if you feel a script plugin is still important please send an email to snappy-app-devel so a wider audience can provide opinions. |
sergiusens
closed this
Mar 31, 2016
LefterisJP
commented
Mar 31, 2016
|
@sergiusens Too bad :( I still think this is important but it's not my call what goes in snapcraft core. I will keep it as a local plugin on my snap then. Thank you for your time. |
LefterisJP commentedJan 22, 2016
LP: #1536624