upload/koji: use the new API of kolo/xmlrpc by default

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.

vendor/github.com/kolo/xmlrpc/README.md

@@ -44,11 +44,12 @@ Data types encoding rules:
 * xmlrpc.Base64 encoded to base64;
 * slice encoded to array;
 
-Structs decoded to struct by following rules:
+Structs encoded to struct by following rules:
 
 * all public field become struct members;
 * field name become member name;
 * if field has xmlrpc tag, its value become member name.
+* for fields tagged with `",omitempty"`, empty values are omitted;
 
 Server method can accept few arguments, to handle this case there is
 special approach to handle slice of empty interfaces (`[]interface{}`).

vendor/github.com/kolo/xmlrpc/client.go

@@ -31,7 +31,7 @@ type clientCodec struct {
 	responses map[uint64]*http.Response
 	mutex     sync.Mutex
 
-	response *Response
+	response Response
 
 	// ready presents channel, that is used to link request and it`s response.
 	ready chan uint64
@@ -43,16 +43,16 @@ type clientCodec struct {
 func (codec *clientCodec) WriteRequest(request *rpc.Request, args interface{}) (err error) {
 	httpRequest, err := NewRequest(codec.url.String(), request.ServiceMethod, args)
 
+	if err != nil {
+		return err
+	}
+
 	if codec.cookies != nil {
 		for _, cookie := range codec.cookies.Cookies(codec.url) {
 			httpRequest.AddCookie(cookie)
 		}
 	}
 
-	if err != nil {
-		return err
-	}
-
 	var httpResponse *http.Response
 	httpResponse, err = codec.httpClient.Do(httpRequest)
 
@@ -75,43 +75,39 @@ func (codec *clientCodec) WriteRequest(request *rpc.Request, args interface{}) (
 
 func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) {
 	var seq uint64
-
 	select {
 	case seq = <-codec.ready:
 	case <-codec.close:
 		return errors.New("codec is closed")
 	}
-
-	codec.mutex.Lock()
-	httpResponse := codec.responses[seq]
-	codec.mutex.Unlock()
-
-	if httpResponse.StatusCode < 200 || httpResponse.StatusCode >= 300 {
-		return fmt.Errorf("request error: bad status code - %d", httpResponse.StatusCode)
-	}
-
-	respData, err := ioutil.ReadAll(httpResponse.Body)
-
-	if err != nil {
-		return err
-	}
-
-	httpResponse.Body.Close()
-
-	resp := NewResponse(respData)
-
-	if resp.Failed() {
-		response.Error = fmt.Sprintf("%v", resp.Err())
-	}
-
-	codec.response = resp
-
 	response.Seq = seq
 
 	codec.mutex.Lock()
+	httpResponse := codec.responses[seq]
 	delete(codec.responses, seq)
 	codec.mutex.Unlock()
 
+	defer httpResponse.Body.Close()
+
+	if httpResponse.StatusCode < 200 || httpResponse.StatusCode >= 300 {
+		response.Error = fmt.Sprintf("request error: bad status code - %d", httpResponse.StatusCode)
+		return nil
+	}
+
+	body, err := ioutil.ReadAll(httpResponse.Body)
+	if err != nil {
+		response.Error = err.Error()
+		return nil
+	}
+
+	resp := Response(body)
+	if err := resp.Err(); err != nil {
+		response.Error = err.Error()
+		return nil
+	}
+
+	codec.response = resp
+
 	return nil
 }
 
@@ -119,12 +115,7 @@ func (codec *clientCodec) ReadResponseBody(v interface{}) (err error) {
 	if v == nil {
 		return nil
 	}
-
-	if err = codec.response.Unmarshal(v); err != nil {
-		return err
-	}
-
-	return nil
+	return codec.response.Unmarshal(v)
 }
 
 func (codec *clientCodec) Close() error {

vendor/github.com/kolo/xmlrpc/decoder.go

@@ -130,8 +130,9 @@ func (dec *decoder) decodeValue(val reflect.Value) error {
 				ismap = true
 			} else if checkType(val, reflect.Interface) == nil && val.IsNil() {
 				var dummy map[string]interface{}
-				pmap = reflect.New(reflect.TypeOf(dummy)).Elem()
-				valType = pmap.Type()
+				valType = reflect.TypeOf(dummy)
+				pmap = reflect.New(valType).Elem()
+				val.Set(pmap)
 				ismap = true
 			} else {
 				return err

vendor/github.com/kolo/xmlrpc/encoder.go

@@ -7,9 +7,13 @@ import (
 	"reflect"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 )
 
+// Base64 represents value in base64 encoding
+type Base64 string
+
 type encodeFunc func(reflect.Value) ([]byte, error)
 
 func marshal(v interface{}) ([]byte, error) {
@@ -80,28 +84,34 @@ func encodeValue(val reflect.Value) ([]byte, error) {
 	return []byte(fmt.Sprintf("<value>%s</value>", string(b))), nil
 }
 
-func encodeStruct(val reflect.Value) ([]byte, error) {
+func encodeStruct(structVal reflect.Value) ([]byte, error) {
 	var b bytes.Buffer
 
 	b.WriteString("<struct>")
 
-	t := val.Type()
-	for i := 0; i < t.NumField(); i++ {
-		b.WriteString("<member>")
-		f := t.Field(i)
+	structType := structVal.Type()
+	for i := 0; i < structType.NumField(); i++ {
+		fieldVal := structVal.Field(i)
+		fieldType := structType.Field(i)
 
-		name := f.Tag.Get("xmlrpc")
-		if name == "" {
-			name = f.Name
+		name := fieldType.Tag.Get("xmlrpc")
+		// if the tag has the omitempty property, skip it
+		if strings.HasSuffix(name, ",omitempty") && isZero(fieldVal) {
+			continue
+		}
+		name = strings.TrimSuffix(name, ",omitempty")
+		if name == "" {
+			name = fieldType.Name
 		}
-		b.WriteString(fmt.Sprintf("<name>%s</name>", name))
 
-		p, err := encodeValue(val.FieldByName(f.Name))
+		p, err := encodeValue(fieldVal)
 		if err != nil {
 			return nil, err
 		}
-		b.Write(p)
 
+		b.WriteString("<member>")
+		b.WriteString(fmt.Sprintf("<name>%s</name>", name))
+		b.Write(p)
 		b.WriteString("</member>")
 	}
 

vendor/github.com/kolo/xmlrpc/is_zero.go

@@ -0,0 +1,44 @@
+package xmlrpc
+
+import (
+	"math"
+	. "reflect"
+)
+
+func isZero(v Value) bool {
+	switch v.Kind() {
+	case Bool:
+		return !v.Bool()
+	case Int, Int8, Int16, Int32, Int64:
+		return v.Int() == 0
+	case Uint, Uint8, Uint16, Uint32, Uint64, Uintptr:
+		return v.Uint() == 0
+	case Float32, Float64:
+		return math.Float64bits(v.Float()) == 0
+	case Complex64, Complex128:
+		c := v.Complex()
+		return math.Float64bits(real(c)) == 0 && math.Float64bits(imag(c)) == 0
+	case Array:
+		for i := 0; i < v.Len(); i++ {
+			if !isZero(v.Index(i)) {
+				return false
+			}
+		}
+		return true
+	case Chan, Func, Interface, Map, Ptr, Slice, UnsafePointer:
+		return v.IsNil()
+	case String:
+		return v.Len() == 0
+	case Struct:
+		for i := 0; i < v.NumField(); i++ {
+			if !isZero(v.Field(i)) {
+				return false
+			}
+		}
+		return true
+	default:
+		// This should never happens, but will act as a safeguard for
+		// later, as a default value doesn't makes sense here.
+		panic(&ValueError{"reflect.Value.IsZero", v.Kind()})
+	}
+}

vendor/github.com/kolo/xmlrpc/response.go

@@ -1,6 +1,7 @@
 package xmlrpc
 
 import (
+	"fmt"
 	"regexp"
 )
 
@@ -8,43 +9,32 @@ var (
 	faultRx = regexp.MustCompile(`<fault>(\s|\S)+</fault>`)
 )
 
-type failedResponse struct {
-	Code  int    `xmlrpc:"faultCode"`
-	Error string `xmlrpc:"faultString"`
+// FaultError is returned from the server when an invalid call is made
+type FaultError struct {
+	Code   int    `xmlrpc:"faultCode"`
+	String string `xmlrpc:"faultString"`
 }
 
-func (r *failedResponse) err() error {
-	return &xmlrpcError{
-		code: r.Code,
-		err:  r.Error,
+// Error implements the error interface
+func (e FaultError) Error() string {
+	return fmt.Sprintf("Fault(%d): %s", e.Code, e.String)
+}
+
+type Response []byte
+
+func (r Response) Err() error {
+	if !faultRx.Match(r) {
+		return nil
 	}
-}
-
-type Response struct {
-	data []byte
-}
-
-func NewResponse(data []byte) *Response {
-	return &Response{
-		data: data,
-	}
-}
-
-func (r *Response) Failed() bool {
-	return faultRx.Match(r.data)
-}
-
-func (r *Response) Err() error {
-	failedResp := new(failedResponse)
-	if err := unmarshal(r.data, failedResp); err != nil {
+	var fault FaultError
+	if err := unmarshal(r, &fault); err != nil {
 		return err
 	}
-
-	return failedResp.err()
+	return fault
 }
 
-func (r *Response) Unmarshal(v interface{}) error {
-	if err := unmarshal(r.data, v); err != nil {
+func (r Response) Unmarshal(v interface{}) error {
+	if err := unmarshal(r, v); err != nil {
 		return err
 	}
 

vendor/github.com/kolo/xmlrpc/test_server.rb

@@ -15,9 +15,9 @@ class Service
     x + y
   end
 
-	def error
-		raise XMLRPC::FaultException.new(500, "Server error")
-	end
+  def error
+    raise XMLRPC::FaultException.new(500, "Server error")
+  end
 end
 
 server = XMLRPC::Server.new 5001, 'localhost'

vendor/github.com/kolo/xmlrpc/xmlrpc.go

@@ -1,19 +0,0 @@
-package xmlrpc
-
-import (
-	"fmt"
-)
-
-// xmlrpcError represents errors returned on xmlrpc request.
-type xmlrpcError struct {
-	code int
-	err  string
-}
-
-// Error() method implements Error interface
-func (e *xmlrpcError) Error() string {
-	return fmt.Sprintf("error: \"%s\" code: %d", e.err, e.code)
-}
-
-// Base64 represents value in base64 encoding
-type Base64 string

vendor/modules.txt

@@ -119,7 +119,7 @@ github.com/gophercloud/gophercloud/pagination
 github.com/jmespath/go-jmespath
 # github.com/julienschmidt/httprouter v1.2.0
 github.com/julienschmidt/httprouter
-# github.com/kolo/xmlrpc v0.0.0-20190417161013-de6d879202d7
+# github.com/kolo/xmlrpc v0.0.0-20200310150728-e0350524596b
 github.com/kolo/xmlrpc
 # github.com/kr/pretty v0.1.0
 github.com/kr/pretty