Skip to content

Commit

Permalink
upload/koji: use the new API of kolo/xmlrpc by default
Browse files Browse the repository at this point in the history
Fedora 33 ships the new API so let's do the switch now.

But... this would break older Fedoras because they only have the old API,
right?

We have the following options:

1) Ship xmlrpc compat package to Fedora 33+. This would mean that we delay the API switch till F32 EOL. This would be the most elegant solution, yet it has two issues: a) We will surely not be able to deliver the compat package before F33 Final Freeze. b) It's an extra and annoying work.

2) Downstream patch. No.

3) Use build constraints and have two versions of our code for both different
   API.

I chose solution #3. It has an issue though:

%gobuild macro already passes -tags argument to go build. Therefore the
following line fails because it's not possible to use -tags more than once:

%gobuild -tags kolo_xmlrpc_oldapi ...

Therefore I had to come up with manual tinkering with the build constraints
in the spec file. This is pretty ugly but I like that:

1) Go code is actually clean, no weird magic is happening there.
2) We can still ship our software to Fedora/RHEL as we used to
   (no downstream patches)
3) All downstreams can use the upstream spec file directly.

Note that this doesn't affect RHEL in any way as it uses vendored libraries.
  • Loading branch information
ondrejbudai authored and msehnout committed Oct 14, 2020
1 parent d323451 commit a67baf5
Show file tree
Hide file tree
Showing 15 changed files with 177 additions and 95 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
github.com/google/uuid v1.1.1
github.com/gophercloud/gophercloud v0.11.0
github.com/julienschmidt/httprouter v1.2.0
github.com/kolo/xmlrpc v0.0.0-20190417161013-de6d879202d7
github.com/kolo/xmlrpc v0.0.0-20200310150728-e0350524596b
github.com/labstack/echo/v4 v4.1.11
github.com/pkg/errors v0.9.1 // indirect
github.com/stretchr/testify v1.5.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5i
github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/julienschmidt/httprouter v1.2.0 h1:TDTW5Yz1mjftljbcKqRcrYhd4XeOoI98t+9HbQbYf7g=
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
github.com/kolo/xmlrpc v0.0.0-20190417161013-de6d879202d7 h1:kL2yi3DjwkRWFgKwD5COyl4XMLKhfOvqck4xyis7EIw=
github.com/kolo/xmlrpc v0.0.0-20190417161013-de6d879202d7/go.mod h1:o03bZfuBwAXHetKXuInt4S7omeXUu62/A845kiycsSQ=
github.com/kolo/xmlrpc v0.0.0-20200310150728-e0350524596b h1:DzHy0GlWeF0KAglaTMY7Q+khIFoG8toHP+wLFBVBQJc=
github.com/kolo/xmlrpc v0.0.0-20200310150728-e0350524596b/go.mod h1:o03bZfuBwAXHetKXuInt4S7omeXUu62/A845kiycsSQ=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
Expand Down
14 changes: 14 additions & 0 deletions golang-github-osbuild-composer.spec
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ Provides: weldr
%goprep
%endif

%if 0%{?fedora} && 0%{?fedora} <= 32
# Fedora 32 and older ships a different kolo/xmlrpc API. We cannot specify
# build tags in gobuild macro because the macro itself specifies build tags.
# and -tags argument cannot be used more than once.
# Therefore, this ugly hack with build tags switcharoo is required.
# Remove when F32 is EOL.

# Remove the build constraint from the wrapper of the old API
sed -i "s$// +build kolo_xmlrpc_oldapi$// +build !kolo_xmlrpc_oldapi$" internal/upload/koji/xmlrpc-response-oldapi.go

# Add a build constraint to the wrapper of the new API
sed -i "s$// +build !kolo_xmlrpc_oldapi$// +build kolo_xmlrpc_oldapi$" internal/upload/koji/xmlrpc-response.go
%endif

%build
%if 0%{?rhel}
GO_BUILD_PATH=$PWD/_build
Expand Down
4 changes: 3 additions & 1 deletion internal/upload/koji/xmlrpc-response-oldapi.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// This files provides a wrapper around kolo/xmlrpc response handling.
// +build kolo_xmlrpc_oldapi
//
// This file provides a wrapper around kolo/xmlrpc response handling.
//
// Commit e3ad6d89 of the xmlrpc library changed the API of response handling.
// This means that different APIs are available in Fedora 32 and 33 (it does
Expand Down
34 changes: 34 additions & 0 deletions internal/upload/koji/xmlrpc-response.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// +build !kolo_xmlrpc_oldapi
//
// This file provides a wrapper around kolo/xmlrpc response handling.
//
// Commit e3ad6d89 of the xmlrpc library changed the API of response handling.
// This means that different APIs are available in Fedora 32 and 33 (it does
// not matter for RHEL as uses vendored libraries).
// This wrapper allows us to use both xmlrpc's APIs using buildflags.
//
// This file is a wrapper for xmlrpc equal or newer than e3ad6d89.

package koji

import (
"fmt"

"github.com/kolo/xmlrpc"
)

// processXMLRPCResponse is a wrapper around kolo/xmlrpc
func processXMLRPCResponse(body []byte, reply interface{}) error {
resp := xmlrpc.Response(body)

if resp.Err() != nil {
return fmt.Errorf("xmlrpc server returned an error: %v", resp.Err())
}

err := resp.Unmarshal(reply)
if err != nil {
return fmt.Errorf("cannot unmarshal the xmlrpc response: %v", err)
}

return nil
}
14 changes: 14 additions & 0 deletions osbuild-composer.spec
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,20 @@ Provides: golang-github-osbuild-composer = %{version}-%{release}
%goprep
%endif

%if 0%{?fedora} && 0%{?fedora} <= 32
# Fedora 32 and older ships a different kolo/xmlrpc API. We cannot specify
# build tags in gobuild macro because the macro itself specifies build tags.
# and -tags argument cannot be used more than once.
# Therefore, this ugly hack with build tags switcharoo is required.
# Remove when F32 is EOL.

# Remove the build constraint from the wrapper of the old API
sed -i "s$// +build kolo_xmlrpc_oldapi$// +build !kolo_xmlrpc_oldapi$" internal/upload/koji/xmlrpc-response-oldapi.go

# Add a build constraint to the wrapper of the new API
sed -i "s$// +build !kolo_xmlrpc_oldapi$// +build kolo_xmlrpc_oldapi$" internal/upload/koji/xmlrpc-response.go
%endif

%build
%if 0%{?rhel}
GO_BUILD_PATH=$PWD/_build
Expand Down
3 changes: 2 additions & 1 deletion vendor/github.com/kolo/xmlrpc/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 19 additions & 28 deletions vendor/github.com/kolo/xmlrpc/client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions vendor/github.com/kolo/xmlrpc/decoder.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 20 additions & 10 deletions vendor/github.com/kolo/xmlrpc/encoder.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 44 additions & 0 deletions vendor/github.com/kolo/xmlrpc/is_zero.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a67baf5

Please sign in to comment.