go plugin: Support local sources #97

Merged
merged 1 commit into from Nov 16, 2015

Conversation

Projects
None yet
3 participants
Collaborator

sergiusens commented Nov 13, 2015

This also avoids the error of checking for : in self.options.source

Closes LP: #1499240
Closes LP: #1515132

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

snapcraft/plugins/go.py
+ - go-packages:
+ (list of strings)
+ Go packages to fetch, these must be a "main" package. Dependencies
+ are pulled in automatically by `go get`.
@elopio

elopio Nov 14, 2015

Member

It would be good here to put the 3 formats that the plugin supports.

@sergiusens

sergiusens Nov 14, 2015

Collaborator

sorry, I screwed up the commits, this should not be here.

def pull(self):
# use -d to only download (build will happen later)
# use -t to also get the test-deps
- self._run(['go', 'get', '-t', '-d', self.fullname])
+ super().pull()
@elopio

elopio Nov 14, 2015

Member

So if the source is git://name/project, will this get the repo two times?
first pull will download it to src, and then _local_pull will download it to src/go/src?
I'm not sure if I'm missing something.

@sergiusens

sergiusens Nov 14, 2015

Collaborator

On Fri, Nov 13, 2015 at 9:30 PM, Leo Arias notifications@github.com wrote:

In snapcraft/plugins/go.py
ubuntu-core#97 (comment):

 def pull(self):
     # use -d to only download (build will happen later)
     # use -t to also get the test-deps
  •    self._run(['go', 'get', '-t', '-d', self.fullname])
    
  •    super().pull()
    

So if the source is git://name/project, will this get the repo two times?
first pull will download it to src, and then _local_pull will download it
to src/go/src?
I'm not sure if I'm missing something.

super().pull() will bring in the source (git://name/project)

self._local_pull() brings in the dependencies for that source.

examples/godd/godd/godd.go
@@ -0,0 +1,243 @@
+package main
@chipaca

chipaca Nov 14, 2015

Member

no copyright header?

examples/godd/godd/godd.go
+ return strconv.Atoi(s)
+ }
+
+ // dd supports suffixes via two chars like "kB"
@chipaca

chipaca Nov 14, 2015

Member

missing P, E, Z, and Y suffixes :-P

examples/godd/godd/udev/udev.go
+ result[i] = device
+ l = l.next
+ }
+ C.g_list_free(l)
@chipaca

chipaca Nov 14, 2015

Member

wouldn't doing this in a defer be better? Ie,

l := C.g_udev_client_query_by_subsystem(c.p, (*C.gchar)(C.CString(subsystem)))
defer C.g_list_free(l)
@chipaca

chipaca Nov 14, 2015

Member

(except without the leak)

examples/godd/godd/udev/udev.go
+}
+
+func (c *Client) QueryBySubsystem(subsystem string) []Device {
+ l := C.g_udev_client_query_by_subsystem(c.p, (*C.gchar)(C.CString(subsystem)))
@chipaca

chipaca Nov 14, 2015

Member

aren't you leaking that Cstring?

examples/godd/godd/udev/udev.go
+ // convert go to char **
+ cs := make([]*C.gchar, len(subsystems)+1)
+ for i := range subsystems {
+ cs[i] = (*C.gchar)(C.CString(subsystems[i]))
@chipaca

chipaca Nov 14, 2015

Member

leaking the string

examples/godd/godd/udev/udev.go
+}
+
+func (d *Device) GetSysfsAttr(name string) string {
+ res := C.g_udev_device_get_sysfs_attr(d.p, (*C.gchar)(C.CString(name)))
@chipaca

chipaca Nov 14, 2015

Member

leaking the string?

examples/godd/godd/udev/udev.go
+}
+
+func (d *Device) GetProperty(name string) string {
+ res := C.g_udev_device_get_property(d.p, (*C.gchar)(C.CString(name)))
Collaborator

sergiusens commented Nov 15, 2015

Thanks @chipaca for the review, I just pulled the upstream in (from @mvo5) to prove local collocated snapcraft.yaml and source code.

I'll replace it with a simple hello world I guess and mayb @mvo5 can fix upstream ;-)

If there is no issue with the python code I am happy as a hippo 🙉

go plugin: Support local sources
This also avoids the error of checking for `:` in self.options.source

Closes LP: #1499240
Closes LP: #1515132

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Member

chipaca commented Nov 16, 2015

👍

sergiusens added a commit that referenced this pull request Nov 16, 2015

Merge pull request #97 from sergiusens/local-go
go plugin: Support local sources

Closes LP: #1499240
Closes LP: #1515132

@sergiusens sergiusens merged commit 7e6dc4b into snapcore:master Nov 16, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 91.487%
Details

@sergiusens sergiusens deleted the sergiusens:local-go branch Nov 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment