Bug 1532364 - Review Request: rpcsvc-proto - RPC protocol definition
Summary: Review Request: rpcsvc-proto - RPC protocol definition
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Igor Raits
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1531540
TreeView+ depends on / blocked
 
Reported: 2018-01-08 18:47 UTC by Steve Dickson
Modified: 2018-08-07 12:13 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-08-07 12:13:27 UTC
Type: ---
Embargoed:
ignatenko: fedora-review+


Attachments (Terms of Use)

Description Steve Dickson 2018-01-08 18:47:11 UTC
Spec URL: http://people.redhat.com/steved/rpcsvc-proto/rpcsvc-proto.spec
SRPM URL: http://people.redhat.com/steved/rpcsvc-proto/rpcsvc-proto-1.3-0.src.rpm
Description: 
The rpcsvc-proto package includes several rpcsvc header files
and RPC protocol definitions from SunRPC sources (as shipped with
glibc).

Fedora Account System Username: steved

Comment 1 Igor Gnatenko 2018-01-08 18:51:32 UTC
> License:        BSD-3-Clause
License: BSD

> Source:         %{name}-%{version}.tar.bz2
How archive is obtained?

> Group:          System/Libraries
Group tag is deprecated

> make %{?_smp_mflags}
%make_build

> make %{?_smp_mflags} DESTDIR=%{buildroot} install
%make_install

> %defattr(-,root,root)
Deprecated

> %dir %{_includedir}/tirpc
> %dir %{_includedir}/tirpc/rpcsvc
> %{_includedir}/tirpc/rpcsvc/*
%{_includedir}/tirpc/

> %changelog
Changelog is missing

> BuildRoot:      %{_tmppath}/%{name}-%{version}-build
Deprecated

> Release:        0
Release:        1%{?dist}

Comment 2 Igor Gnatenko 2018-01-08 18:53:04 UTC
> Provides:       glibc-devel:%{_bindir}/rpcgen
This won't work. You should coordinate adding Obsoletes with glibc maintainers so this package gets pulled in automatically.

Comment 3 Steve Dickson 2018-01-08 18:58:11 UTC
(In reply to Igor Gnatenko from comment #2)
> > Provides:       glibc-devel:%{_bindir}/rpcgen
> This won't work. You should coordinate adding Obsoletes with glibc
> maintainers so this package gets pulled in automatically.

I have been... Please see 
   https://bugzilla.redhat.com/show_bug.cgi?id=1531540#c7

For now nfs-utils will have a dependency on a new glibc-rpcgen 
subpackage to pull in rpcgen... once this package is ready
I'm planing on just switch the dependency...

Comment 4 Florian Weimer 2018-01-08 19:28:14 UTC
Provides: rpcgen

should be good enough because the package has never been part of a release.  I will simply drop the glibc-rpcgen subpackage once this is in.

Obsoletes: glibc-rpcgen

would be okay as well.

Note that the license should be “BSD and LGPLv2+”, as the sources were initially copied from glibc and not the original Sun sources.

Comment 5 Igor Gnatenko 2018-01-08 19:29:59 UTC
(In reply to Florian Weimer from comment #4)
> Provides: rpcgen
> 
> should be good enough because the package has never been part of a release. 
> I will simply drop the glibc-rpcgen subpackage once this is in.
> 
> Obsoletes: glibc-rpcgen

Non-versioned obsoletes are bad and prohibited, you need to add version to it.

Comment 6 Steve Dickson 2018-01-08 22:06:02 UTC
(In reply to Igor Gnatenko from comment #1)
> > License:        BSD-3-Clause
> License: BSD
License:        BSD and LGPLv2

> 
> > Source:         %{name}-%{version}.tar.bz2
> How archive is obtained?
Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/%{name}-%{version}.tar.gz

> 
> > Group:          System/Libraries
> Group tag is deprecated
Group:          System Environment/Libraries

> 
> > make %{?_smp_mflags}
> %make_build
Done.

> 
> > make %{?_smp_mflags} DESTDIR=%{buildroot} install
> %make_install
Done 

> 
> > %defattr(-,root,root)
> Deprecated
Removed.

> 
> > %changelog
> Changelog is missing
* Mon Jan  8 2018 Steve Dickson <steved>  1.3-1
- Initial commit

> 
> > BuildRoot:      %{_tmppath}/%{name}-%{version}-build
> Deprecated
Gone.

> 
> > Release:        0
> Release:        1%{?dist}
Done.

> 
> > %dir %{_includedir}/tirpc
> > %dir %{_includedir}/tirpc/rpcsvc
> > %{_includedir}/tirpc/rpcsvc/*
> %{_includedir}/tirpc/
This is very curious rule... 

First of all theses header files needed to stay
in /usr/include/rpcsvc since that is where they are
today. We move them to tirpc/rpcsvc or tirpc that
is going to break the API.

Doing the build/install from the upstream tarball these
header files are install in /usr/include/rpcsvc which
makes sense. Let me ask the maintainer why he is move
them.

Also these files are currently in glibc-headers...
Florian, are these files going to moved out of that package? 


http://people.redhat.com/steved/rpcsvc-proto has been updated.

Comment 7 Igor Gnatenko 2018-01-08 22:18:15 UTC
* Don't specify Group tag at all
* Use https for URL

> This is very curious rule... 
That's how 99% of packages are done, just listing all files is too verbose and doesn't help anyone.

Comment 8 Florian Weimer 2018-01-09 08:20:34 UTC
(In reply to Steve Dickson from comment #6)
> Also these files are currently in glibc-headers...
> Florian, are these files going to moved out of that package? 

Most of the headers are already gone:

# ls /usr/include/rpcsvc/
nis_callback.h  nis.h     nis_object.x  nis.x     yp.h       ypupd.h
nis_callback.x  nislib.h  nis_tags.h    ypclnt.h  yp_prot.h  yp.x
# rpm -q glibc-headers
glibc-headers-2.26.9000-37.fc28.x86_64

The other headers are NIS headers and will be provided by packaging the split-out NIS packages (separate from this effort).

I think it is sufficient to declare a conflicts:

Conflicts: glibc-headers < 2.26.9000-36
Conflicts: glibc-common < 2.26.9000-36

(The second one is needed to deal with the rpcgen binary itself.)

Igor, does this look right to you?

Comment 9 Igor Gnatenko 2018-01-09 09:53:48 UTC
(In reply to Florian Weimer from comment #8)
> [...]
> I think it is sufficient to declare a conflicts:
> 
> Conflicts: glibc-headers < 2.26.9000-36
> Conflicts: glibc-common < 2.26.9000-36
> 
> (The second one is needed to deal with the rpcgen binary itself.)
> 
> Igor, does this look right to you?

If you don't want to automatically install this package on update from old glibc, then yes :)

Comment 10 Steve Dickson 2018-01-09 16:50:43 UTC
(In reply to Igor Gnatenko from comment #7)
> * Don't specify Group tag at all
Removed 
> * Use https for URL
Updated

(In reply to Florian Weimer from comment #8)
> I think it is sufficient to declare a conflicts:
> 
> Conflicts: glibc-headers < 2.26.9000-36
> Conflicts: glibc-common < 2.26.9000-36
Added.

http://people.redhat.com/steved/rpcsvc-proto has been updated.

Comment 11 Steve Dickson 2018-01-09 16:52:56 UTC
I also has to add this patch:

diff -up rpcsvc-proto-1.3/autogen.sh.orig rpcsvc-proto-1.3/autogen.sh
--- rpcsvc-proto-1.3/autogen.sh.orig    2017-11-24 04:14:15.000000000 -0500
+++ rpcsvc-proto-1.3/autogen.sh 2018-01-09 11:34:58.809781787 -0500
@@ -1,7 +1,7 @@
 #!/bin/sh -x

 rm -fv config.sub config.guess config.h.in
-#aclocal -I m4
+aclocal
 autoheader
 #libtoolize --automake --copy
 automake --add-missing --copy --force

to avoid these errors:

/usr/share/automake-1.15/am/depend2.am: error: am__fastdepCC does not appear in AM_CONDITIONAL
/usr/share/automake-1.15/am/depend2.am:   The usual way to define 'am__fastdepCC' is to add 'AC_PROG_CC'
/usr/share/automake-1.15/am/depend2.am:   to 'configure.ac' and run 'aclocal' and 'autoconf' again
/usr/share/automake-1.15/am/depend2.am: error: AMDEP does not appear in AM_CONDITIONAL
/usr/share/automake-1.15/am/depend2.am:   The usual way to define 'AMDEP' is to add one of the compiler tests
/usr/share/automake-1.15/am/depend2.am:     AC_PROG_CC, AC_PROG_CXX, AC_PROG_OBJC, AC_PROG_OBJCXX,
/usr/share/automake-1.15/am/depend2.am:     AM_PROG_AS, AM_PROG_GCJ, AM_PROG_UPC
/usr/share/automake-1.15/am/depend2.am:   to 'configure.ac' and run 'aclocal' and 'autoconf' again

Comment 12 Florian Weimer 2018-01-09 16:54:56 UTC
I think you still need to fix the Provides: line (and perhaps add Provides: /usr/bin/rpcgen as well).

Comment 13 Florian Weimer 2018-01-09 16:55:27 UTC
And when I last built the proposed package, it failed because there was no ./configure (missing autoreconf?).

Comment 14 Steve Dickson 2018-01-09 18:08:54 UTC
(In reply to Florian Weimer from comment #12)
> I think you still need to fix the Provides: line (and perhaps add Provides:
> /usr/bin/rpcgen as well).

Is the the fix you are suggesting?

< Provides:       glibc-devel:%{_bindir}/rpcgen
---
> Provides:       %{_bindir}/rpcgen

Comment 15 Steve Dickson 2018-01-09 18:11:44 UTC
(In reply to Florian Weimer from comment #13)
> And when I last built the proposed package, it failed because there was no
> ./configure (missing autoreconf?).

I add the running of the autoconf.sh file to the %build 
%build
./autogen.sh
%configure
%make_build

for the ./configure to be created... 

It seems to build now (at least locally)
https://paste.fedoraproject.org/paste/uW1qRXzmqgWDAAcesWU81A

Comment 16 Florian Weimer 2018-01-09 18:39:18 UTC
(In reply to Steve Dickson from comment #14)
> (In reply to Florian Weimer from comment #12)
> > I think you still need to fix the Provides: line (and perhaps add Provides:
> > /usr/bin/rpcgen as well).
> 
> Is the the fix you are suggesting?
> 
> < Provides:       glibc-devel:%{_bindir}/rpcgen
> ---
> > Provides:       %{_bindir}/rpcgen

I'd prfer:

Provides: rpcgen
Provides: %{_bindir}/rpcgen

Some packages probably use

BuildRequires: rpcgen

and we should support that.

Igor, does this look okay to you?

Comment 17 Steve Dickson 2018-01-09 18:51:16 UTC
(In reply to Florian Weimer from comment #16)
> (In reply to Steve Dickson from comment #14)
> > (In reply to Florian Weimer from comment #12)
> > > I think you still need to fix the Provides: line (and perhaps add Provides:
> > > /usr/bin/rpcgen as well).
> > 
> > Is the the fix you are suggesting?
> > 
> > < Provides:       glibc-devel:%{_bindir}/rpcgen
> > ---
> > > Provides:       %{_bindir}/rpcgen
> 
> I'd prfer:
> 
> Provides: rpcgen
> Provides: %{_bindir}/rpcgen
> 
> Some packages probably use
> 
> BuildRequires: rpcgen
> 
> and we should support that.
I agree... spec file updated.

Comment 18 Igor Gnatenko 2018-01-09 18:59:51 UTC
> %dir %{_includedir}/rpcsvc
> %{_includedir}/rpcsvc/*
%{_includedir}/rpcsvc/

> %doc COPYING
%license COPYING

* Group tag is still not removed
* License is not included with rpcgen

Comment 19 Steve Dickson 2018-01-10 14:09:50 UTC
(In reply to Igor Gnatenko from comment #18)
> > %dir %{_includedir}/rpcsvc
> > %{_includedir}/rpcsvc/*
> %{_includedir}/rpcsvc/
Removed the '*'
 %dir %{_includedir}/rpcsvc
-%{_includedir}/rpcsvc/*
+%{_includedir}/rpcsvc/

> 
> > %doc COPYING
> %license COPYING
Changed the %doc to %license
 %files devel
-%doc COPYING
+%license COPYING

> 
> * Group tag is still not removed
Removed all Group:
 %package devel
 Summary:        RPC protocol definitions
-Group:          Development/Libraries/C and C++

 %package -n rpcgen
 Summary:        RPC protocol compiler
-Group:          Development/Tools/Other

> * License is not included with rpcgen
Added license to rpcgen package 

+License:        BSD and LGPLv2+
 Provides:       rpcgen

Also removed the patch to autogen.sh since upstream
accepted the patch which caused the Version  to be bumped.
-Version:        1.3
+Version:        1.3.1

-Patch100: rpcsvc-proto-1.3-autogen.diff
-
 %prep
-%autosetup -p1
+%setup -q

http://people.redhat.com/steved/rpcsvc-proto has been updated.

Comment 20 Florian Weimer 2018-01-12 10:56:39 UTC
What is the status here?  Thanks.

Comment 21 Steve Dickson 2018-01-12 12:50:18 UTC
(In reply to Florian Weimer from comment #20)
> What is the status here?  Thanks.

IDK... what is the next step?

Comment 22 Charalampos Stratakis 2018-01-12 16:05:53 UTC
The headers were removed from glibc but the package that provides the headers now is still in review.

python2 and python3 use those header to build the nis module and currently both FTBFS which blocks us on other more important fixes that should be commited soon.

Could the review proceed here?

Comment 23 Florian Weimer 2018-01-12 16:10:42 UTC
(In reply to Charalampos Stratakis from comment #22)
> The headers were removed from glibc but the package that provides the
> headers now is still in review.

The headers are provided by libtirpc-devel.  You should add a BuildRequires for it.

Comment 24 Igor Gnatenko 2018-01-13 08:09:48 UTC
Source URL is broken...

> Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/%{name}-%{version}.tar.gz
Should be Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/v%{version/%{name}-%{version}.tar.gz

> Provides:       %{_bindir}/rpcgen
Remove this, you already have file.

> %dir %{_includedir}/rpcsvc
This is still not removed

> License:        BSD and LGPLv2+
This is not needed for subpackage

Once this is fixed, I will approve.

Comment 25 Steve Dickson 2018-01-13 14:41:25 UTC
(In reply to Igor Gnatenko from comment #24)
> Source URL is broken...
> 
> > Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/%{name}-%{version}.tar.gz
> Should be Source0:       
> https://github.com/thkukuk/rpcsvc-proto/archive/v%{version/%{name}-
> %{version}.tar.gz
There is an unbalance 'v%{version' in this link... I think this is what you mean
https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}.tar.gz

But make note of the tar balls in previous release. The were all
in the %{name}-%{version}.tar.gz format so I'm thinking this might
be a typo by the upstream maintainer.

> 
> > Provides:       %{_bindir}/rpcgen
> Remove this, you already have file.
Gone.

> 
> > %dir %{_includedir}/rpcsvc
> This is still not removed
Gone.
> 
> > License:        BSD and LGPLv2+
> This is not needed for subpackage
Gone.


-- rpcsvc-proto.spec.v6	2018-01-10 09:06:41.902749055 -0500
+++ rpcsvc-proto.spec.v7	2018-01-13 09:37:53.694294293 -0500
@@ -22,7 +22,7 @@ Release:        1%{?dist}
 Summary:        RPC protocol definitions
 License:        BSD and LGPLv2+
 Url:            https://github.com/thkukuk/rpcsvc-proto
-Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/%{name}-%{version}.tar.gz
+Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}.tar.gz
 
 Conflicts: glibc-headers < 2.26.9000-36
 Conflicts: glibc-common < 2.26.9000-36
@@ -42,9 +42,7 @@ glibc).
 
 %package -n rpcgen
 Summary:        RPC protocol compiler
-License:        BSD and LGPLv2+
 Provides:       rpcgen
-Provides:       %{_bindir}/rpcgen
 
 %description -n rpcgen
 rpcgen is a tool that generates C code to implement an RPC protocol.
@@ -64,7 +62,6 @@ The input to rpcgen is a language simila
 
 %files devel
 %license COPYING
-%dir %{_includedir}/rpcsvc
 %{_includedir}/rpcsvc/
 
 %files -n rpcgen

spec file under http://people.redhat.com/steved/rpcsvc-proto has been updated

Comment 26 Igor Gnatenko 2018-01-13 14:46:01 UTC
> +Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}.tar.gz
I simply forgot to add enclosing brace, so it should be

https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}/%{name}-%{version}.tar.gz

Comment 27 Steve Dickson 2018-01-13 14:49:36 UTC
(In reply to Igor Gnatenko from comment #26)
> > +Source0:        https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}.tar.gz
> I simply forgot to add enclosing brace, so it should be
> 
> https://github.com/thkukuk/rpcsvc-proto/archive/v%{version}/%{name}-
> %{version}.tar.gz

spec file under http://people.redhat.com/steved/rpcsvc-proto has been updated

Comment 28 Igor Gnatenko 2018-01-13 15:02:45 UTC
APPROVED.

Comment 29 Igor Gnatenko 2018-01-13 15:22:14 UTC
One more thing, add BuildRequires: gcc

Comment 30 Steve Dickson 2018-01-13 16:32:59 UTC
(In reply to Igor Gnatenko from comment #29)
> One more thing, add BuildRequires: gcc
Done!

spec file under http://people.redhat.com/steved/rpcsvc-proto has been updated

What is the next step?

Comment 31 Steve Dickson 2018-01-15 19:11:51 UTC
Waiting for approval:

fedrepo-req: https://pagure.io/releng/fedora-scm-requests/issue/4175
fedrepo-req-branch: https://pagure.io/releng/fedora-scm-requests/issue/4176

Comment 32 Gwyn Ciesla 2018-01-15 22:20:23 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rpcsvc-proto

Comment 33 Steve Dickson 2018-01-17 21:49:49 UTC
The koji build
https://koji.fedoraproject.org/koji/taskinfo?taskID=24249811

What is it going to take to get his built package on he
build root so nfs-utils can use it?

Comment 34 Florian Weimer 2018-01-18 07:54:20 UTC
(In reply to Steve Dickson from comment #33)
> The koji build
> https://koji.fedoraproject.org/koji/taskinfo?taskID=24249811
> 
> What is it going to take to get his built package on he
> build root so nfs-utils can use it?

The build was tagged into the buildroot at the end of the task.  By default, this happens automatically for Fedora builds.

Comment 35 Steve Dickson 2018-01-19 16:19:48 UTC
How come the rawhide repo does not have these rpms 

# dnf list rpcsvc-proto
Last metadata expiration check: 0:27:02 ago on Fri 19 Jan 2018 10:46:40 AM EST.
Error: No matching Packages to list

# dnf list rpcgen
Last metadata expiration check: 0:27:12 ago on Fri 19 Jan 2018 10:46:40 AM EST.
Error: No matching Packages to list

the repo

[rawhide]
name=Fedora - Rawhide - Developmental packages for the next Fedora release
failovermethod=priority
#baseurl=http://download.fedoraproject.org/pub/fedora/linux/development/rawhide//Everything/$basearch/os/
metalink=https://mirrors.fedoraproject.org/metalink?repo=rawhide&arch=$basearch
enabled=1

Comment 36 Florian Weimer 2018-01-19 16:33:00 UTC
This works for me:

mock -r fedora-rawhide-x86_64 --dnf-cmd --enablerepo=local install rpcgen rpcsvc-proto-devel

There hasn't been a rawhide compose since the package was built, and there isn't a package called rpcsvc-proto.

Comment 37 Igor Raits 2018-07-29 16:08:53 UTC
Any news here?

Comment 38 Steve Dickson 2018-08-07 11:03:35 UTC
(In reply to Igor Gnatenko from comment #37)
> Any news here?

What do I need to do here???

Comment 39 Igor Raits 2018-08-07 11:20:11 UTC
(In reply to Steve Dickson from comment #38)
> (In reply to Igor Gnatenko from comment #37)
> > Any news here?
> 
> What do I need to do here???

You have submitted review, but you never closed this bug.


Note You need to log in before you can comment on or make changes to this bug.