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

bump Go to 1.4 #9615

Merged
merged 3 commits into from
Dec 22, 2014
Merged

bump Go to 1.4 #9615

merged 3 commits into from
Dec 22, 2014

Conversation

unclejack
Copy link
Contributor

This bumps the Go version to 1.4.

Please note that this version of Go breaks gosqlite and it also breaks mattn/go-sqlite3 when used with Docker.

This PR is also meant to be the place where we discuss everything related to this bump.

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 11, 2014

And if I understand correctly - docker binary became always dynamically linked with glibc.

@vbatts
Copy link
Contributor

vbatts commented Dec 11, 2014

wait, @unclejack we don't want to bump yet, just to talk about it?

@unclejack
Copy link
Contributor Author

@vbatts Yes, but this is better than opening an issue.

@vbatts
Copy link
Contributor

vbatts commented Dec 11, 2014

understood

@prologic
Copy link
Contributor

/cc @therealprologic

@unclejack
Copy link
Contributor Author

#9649 is about the issues we're seeing with errors like "database is locked" and "transaction in transaction" errors encountered when using Go 1.4 to build Docker.

That is a major blocker for switching to Go 1.4.

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 18, 2014

golang/go#9380 for reference

@@ -76,6 +76,9 @@ ENV DOCKER_CROSSPLATFORMS \
ENV GOARM 5
RUN cd /usr/local/go/src && bash -xc 'for platform in $DOCKER_CROSSPLATFORMS; do GOOS=${platform%/*} GOARCH=${platform##*/} ./make.bash --no-clean 2>&1; done'

# reinstall standard library with netgo
RUN go clean -i net && go install -tags netgo std
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we could get something official telling us that this really is "the new official way" and that this isn't going to break on us in 1.5 or 1.6. 😢 (I guess that'd have to be in the form of an integration test upstream.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tianon Do you want to open an issue on the Go repo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

golang/go#9369 is an open discussion of netgo 👍

@unclejack
Copy link
Contributor Author

@tiborvass rebased on master

@unclejack
Copy link
Contributor Author

This should be almost good to go now. There's only one failing test (the one with the tabs).

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 20, 2014

@unclejack Yup, can we just swap this symbols to \t right in this PR?

@unclejack
Copy link
Contributor Author

@LK4D4 You can send a PR against my branch if you'd like to do that.

unclejack and others added 3 commits December 21, 2014 13:49
Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com>
Fixes moby#9449

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
Signed-off-by: Alexander Morozov <lk4d4@docker.com>
@LK4D4
Copy link
Contributor

LK4D4 commented Dec 21, 2014

LGTM
Probably we can merge this tomorrow with drone image updating.

@jessfraz
Copy link
Contributor

LGTM

jessfraz pushed a commit that referenced this pull request Dec 22, 2014
@jessfraz jessfraz merged commit d09421a into moby:master Dec 22, 2014
@thaJeztah
Copy link
Member

Great job!

@unclejack unclejack deleted the bump_go_to_1.4 branch December 22, 2014 23:26
@jessfraz
Copy link
Contributor

ping @tianon sorry I thought you had LGTM... eeek :/

@tianon
Copy link
Member

tianon commented Dec 23, 2014

We haven't solved the gofmt issue yet...

@tianon
Copy link
Member

tianon commented Dec 23, 2014

Meaning: now we might merge PRs that have invalid syntax for Go 1.3 or otherwise have our CI complain about PRs that have perfect syntax but it's not Go 1.4 gofmted, so it fails.

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 23, 2014

@tianon if we want so hard to have compat with 1.3, then we know place where syntax changed - empty range cycles. We can rewrite them to always use variable and place comment that this is for 1.3 compatibility.

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 23, 2014

Main problem that now we have no cool travis version separation now :/

@jessfraz
Copy link
Contributor

Hmm I bet we could make jenkins do that...?

On Tue, Dec 23, 2014 at 9:06 AM, Alexander Morozov <notifications@github.com

wrote:

Main problem that now we have no cool travis version separation now :/


Reply to this email directly or view it on GitHub
#9615 (comment).

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 23, 2014

@jfrazelle Yes, we can :) But if we will test only master, then there is chance that there will be broken commit in master.

@tianon
Copy link
Member

tianon commented Dec 23, 2014

@LK4D4 yeah, that'll work, but the CI will still mark such PRs as invalid, and then we have to be manually diligent in checking for this

@LK4D4
Copy link
Contributor

LK4D4 commented Dec 23, 2014

@tianon There is no such code in docker now. So, I think it won't be to hard to trace it in future(it it will be something to trace)

@tianon
Copy link
Member

tianon commented Dec 23, 2014

OK, fair enough

tianon added a commit to tianon/docker-overlay that referenced this pull request Dec 25, 2014
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

7 participants