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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go plugin creating invalid env #161

Merged
merged 1 commit into from Dec 11, 2015

Conversation

jocave
Copy link
Contributor

@jocave jocave commented Dec 9, 2015

@come-maiz
Copy link
Contributor

This makes sense to me, but I would feel safer with a regression test. Have you been able to reduce the issue to a minimum scenario?

@cwayne18
Copy link
Contributor

@ElOpio We haven't been able to reduce it to a minimum scenario quite yet (so far we're only seeing it with building our snap that we'd rather share in public repos yet). Will this block us from landing this? It'd be great to not need a fork of snapcraft to build our snap :)

@come-maiz
Copy link
Contributor

If this is urgent for you, sure, we can land it. But if it can wait, I would prefer to fully understand the issue before making a fix, by writing a regression test.
Can you share the snapcraft with me?

@come-maiz
Copy link
Contributor

Merging. Now I see this is a clear fix for the issue and the test updated ensures the space is in there.
I'll ask Sergio next week why we are sharing a single env. The error happens when wrapping the python3 executable, and CGO_LDFLAGS are not needed in there. I'll keep digging.
Thanks for the report, and thanks for the fix!

come-maiz added a commit that referenced this pull request Dec 11, 2015
@come-maiz come-maiz merged commit 74c81b7 into canonical:master Dec 11, 2015
@kyrofa
Copy link
Contributor

kyrofa commented Jan 20, 2016

This needs to be backported to 1.x.

@jocave jocave deleted the go-ldflags-env-fix branch March 2, 2016 09:55
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants