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

go1.4.2 compatibility #89

Closed
runcom opened this issue Jun 1, 2016 · 9 comments
Closed

go1.4.2 compatibility #89

runcom opened this issue Jun 1, 2016 · 9 comments

Comments

@runcom
Copy link
Member

runcom commented Jun 1, 2016

Building skopeo in RHEL7 with go1.4.2 gives:

$ /usr/local/go1.4.2/go/bin/go build -o skopeo ./cmd/skopeo
# github.com/projectatomic/skopeo/signature
src/github.com/projectatomic/skopeo/signature/json.go:68: dec.Token undefined (type *json.Decoder has no field or method Token)
src/github.com/projectatomic/skopeo/signature/json.go:72: undefined: json.Delim
src/github.com/projectatomic/skopeo/signature/json.go:76: dec.Token undefined (type *json.Decoder has no field or method Token)
src/github.com/projectatomic/skopeo/signature/json.go:80: undefined: json.Delim
src/github.com/projectatomic/skopeo/signature/json.go:103: dec.Token undefined (type *json.Decoder has no field or method Token)

The fastest thing we can do to deal with this is to create a file signature/json_go1.4.2.go (or the like) and put there a build tag to be compiled only with go1.4.2 and lower - and another file to build with go > 1.4.2 (which has the undefined methods above in the errors). We can just move there paranoidUnmarshalJSONObject so we have two version of this function for the different go versions
.
@mtrmac sounds good to you? is paranoidUnmarshalJSONObject fine to be reimplemented w/o using Token and Delim which aren't available in go1.4.2?

@mtrmac
Copy link
Collaborator

mtrmac commented Jun 1, 2016

https://github.com/mtrmac/skopeo/tree/go142 , but please do not merge this. Let those who need to build against the old version apply this manually.

@runcom
Copy link
Member Author

runcom commented Jun 1, 2016

Correct, let's keep this open for a while. I'll close it as soon as we switch to go 1.5+ in at least RHEL
Thanks

@cgwalters
Copy link
Contributor

@runcom
Copy link
Member Author

runcom commented Jun 1, 2016

@cgwalters then I think this patch https://github.com/mtrmac/skopeo/tree/go142 needs to be applied - same as we do for RHEL

@cgwalters
Copy link
Contributor

The patch strategy doesn't really work with the idea of continuous delivery, which is what CAHC is trying to do. At least nontrivial patches to code which is likely to change.

I can freeze for now, but it seems clear that we need to pull in a newer golang. This would be a good on-list discussion.

@runcom
Copy link
Member Author

runcom commented Jun 1, 2016

I can freeze for now, but it seems clear that we need to pull in a newer golang. This would be a good on-list discussion.

great

@cgwalters
Copy link
Contributor

Okay, last commit in CentOS/sig-atomic-buildscripts#85 : CentOS/sig-atomic-buildscripts@93d09a1

BTW, I think this is a cool aspect of rpmdistro-gitoverlay: I can trivially unwind to arbitrary commits both upstream and dist-git. I picked those commits by looking at the last successful build.

mtrmac added a commit to mtrmac/skopeo that referenced this issue Jun 2, 2016
This is currently known to fail due to
containers#89
mtrmac added a commit to mtrmac/skopeo that referenced this issue Jun 3, 2016
This is currently known to fail due to
containers#89
mtrmac added a commit to mtrmac/skopeo that referenced this issue Jun 3, 2016
This is currently known to fail due to
containers#89
@mtrmac
Copy link
Collaborator

mtrmac commented Jun 3, 2016

#47 via #46 will hopefully prevent such breakage to be merged in the future; for now I am rather unhappy with the loss of functionality required in the go142 branch.

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 17, 2016

As of https://rhn.redhat.com/errata/RHSA-2016-1538.html / https://lists.centos.org/pipermail/centos-announce/2016-August/022005.html , CentOS has golang 1.6.3 , so this is no longer relevant.

Attaching a diff showing the necessary code and build machinery changes (and parts of #46), purely for long-term archival before I delete the branches.
go142.patch.txt

@mtrmac mtrmac closed this as completed Aug 17, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants