Bug 658796 - Review Request: ptpd - implements the Precision Time protocol
Review Request: ptpd - implements the Precision Time protocol
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
: 556611 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-01 05:31 EST by Jon Kent
Modified: 2013-04-11 23:47 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-04-11 23:47:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
lemenkov: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jon Kent 2010-12-01 05:31:02 EST
Spec URL: http://svn2.xp-dev.com/svn/fedora/tags/1.1.0-1/ptpd.spec

SRPM URL: http://svn2.xp-dev.com/svn/fedora/tags/1.1.0-1/ptpd-1.1.0-1.fc14.src.rpm

Description: The PTP daemon (PTPd) implements the Precision Time protocol (PTP) as defined by the relevant IEEE 1588 standard. PTP was developed to provide very precise time coordination of LAN connected computers, synchronising clocks within a ptpd domain within a microsecond.  It is used instead of ntpd, as ntpd synchronisation is only within a second and in some situations this is not tight enough.
Comment 1 Peter Lemenkov 2010-12-01 05:40:22 EST
Jon, is it your first package in Fedora?
Comment 2 Jon Kent 2010-12-01 05:43:56 EST
Peter,

Yes this is.  Done a lot of Red Hat packaging internally at work, but this is my first for Fedora.

Regards
Comment 3 Peter Lemenkov 2010-12-01 05:56:23 EST
I'll review this package and I'll sponsor you (later, after reviewing).
Comment 4 Jon Kent 2010-12-01 06:03:46 EST
Thanks for that, much appreciated.
Comment 5 Peter Lemenkov 2010-12-01 06:09:17 EST
Builds fine in koji for F-14

http://koji.fedoraproject.org/koji/taskinfo?taskID=2636504

Notes:

* The 'Packager' field should be removed entirely.
* Reformat %description text to fin into 80-chars lines.
* This package doesn't honour CFLAGS (see build-log)
* No need to explicitly install doc-files. Just mark them as %doc in the %files section (they will be picked up by rpmbuild automatically from %{_builddir}/%{buildsubdir}
* This line is generally ok: 

cd %{_builddir}/%{name}-%{version}/src

but I'd rather cut it down to "cd src". You're already in %{_builddir}/%{buildsubdir} so no need to append it again.
Comment 6 Peter Lemenkov 2010-12-01 06:14:41 EST
s,fin,fit
Comment 7 Jon Kent 2010-12-01 07:58:20 EST
I've addressed all the above (and updates the svn repo) but the CFLAGS issue as I can't see anything in my rpmbuild output, can you expand pls so I can address this?

Thanks,
Jon
Comment 8 Peter Lemenkov 2010-12-01 08:26:27 EST
(In reply to comment #7)
> I've addressed all the above (and updates the svn repo) but the CFLAGS issue as

You should drop these lines now:

cp -p %{_builddir}/%{name}-%{version}/COPYRIGHT %{buildroot}%{_defaultdocdir}/%{name}-%{version}
cp -p %{_builddir}/%{name}-%{version}/README %{buildroot}%{_defaultdocdir}/%{name}-%{version}
cp -p %{_builddir}/%{name}-%{version}/RELEASE_NOTES %{buildroot}%{_defaultdocdir}/%{name}-%{version}
cp -p %{_builddir}/%{name}-%{version}/ChangeLog %{buildroot}%{_defaultdocdir}/%{name}-%{version}

You already marked these files as %doc so they will be picked up right from the %{_builddir}/%{name}-%{version} by rpmbuild.

Also ther is no need to explicitly mention %{_builddir}/%{name}-%{version} everytime in the %install section - you already in %{_builddir}/%{name}-%{version} so this prefix may be omitted. 


> I can't see anything in my rpmbuild output, can you expand pls so I can address
> this?

Yes, sure.

Take a look at the build log from the koji link above:

http://koji.fedoraproject.org/koji/getfile?taskID=2636505&name=build.log

See - no extra compiler's options were passed to ${CC}. You must explicitly set CFLAGS before make invocation:

CFLAGS="%{optflags}" make ptpd


Another one issue which brings my attention is 3-fold extraction of %{SOURCE0}. This looks somewhat excessive and should be fixed as well. I adwice you to change %prep section as follows:

%prep
# extract first source implicitly and add in fedora init script and sysconfig
%setup -a 1


Also you may just list these two files as %{SOURCE1} and %{SOURCE2} and just copy them directly in the %install section w/o touching them during %prep stage.
Comment 9 Jon Kent 2010-12-01 09:44:32 EST
Thanks for that, its looking much tidier its has to be said.

Put that all together now and push it to svn.

Thanks,
Jon
Comment 10 Bill Nottingham 2010-12-01 15:23:15 EST
See https://bugzilla.redhat.com/show_bug.cgi?id=556611#c9

Adding to FE-Legal.
Comment 11 Jon Kent 2010-12-01 16:19:27 EST
Thats annoying and interesting at the same time as I thought I'd done all the relevant searches before creating this.

From that original package it was withdrawn, as opposed to any official line taken, so I assume this now needs to wait for FE-Legal?
Comment 12 Peter Lemenkov 2010-12-02 07:29:13 EST
(In reply to comment #11)
> Thats annoying and interesting at the same time as I thought I'd done all the
> relevant searches before creating this.
> 
> From that original package it was withdrawn, as opposed to any official line
> taken, so I assume this now needs to wait for FE-Legal?

Yes' we need explicit approval from FE-LEGAL.
Comment 13 Jon Kent 2010-12-02 12:08:53 EST
Ok, no probs.  Putting aside this issue for a moment, is the package itself looking sound now?  I'll understand if you don't want to spend any more time reviewing it, just interesting in the current state of play.

thanks
Comment 14 Peter Lemenkov 2010-12-07 07:19:52 EST
(In reply to comment #13)
> Ok, no probs.  Putting aside this issue for a moment, is the package itself
> looking sound now?  I'll understand if you don't want to spend any more time
> reviewing it, just interesting in the current state of play.
> 
> thanks

Sorry for the delay. Yes, the package now looks good. I'll finish review after the FE-LEGAL will be listed.
Comment 15 Jon Kent 2011-02-21 17:46:28 EST
Hi,

Just a quick little note.  I was wondering how long FE-LEGAL usually take in these matters.

I know it hasn't been that long (just over 2 months), just wanted to say that I'm still here and waiting for those guys to give this the OK.

Regards,

Jon
Comment 16 Tom "spot" Callaway 2011-06-30 13:16:41 EDT
The concerns from the original bug are still applicable, sadly. Closing as CANTFIX.
Comment 17 Jon Stanley 2012-09-24 10:40:22 EDT
Based on progress made in bug 807810, is this still legal blocked?
Comment 18 Tom "spot" Callaway 2012-09-24 10:44:13 EDT
No. Reopening.
Comment 19 Jon Stanley 2012-09-24 10:50:06 EDT
Cool, that's the answer I was hoping for :)

Next, is the original packager still around and interested in this?
Comment 20 Peter Lemenkov 2013-03-22 07:03:16 EDT
Ping! Any news here?
Comment 21 Jon Kent 2013-03-22 08:26:45 EDT
Hi,

Sorry, didn't see that the legal issues have been put to bed on this on.

Happy to continue with this package, just let me know.

Regards,
Jon
Comment 22 Peter Lemenkov 2013-03-22 09:11:07 EDT
Hello!

(In reply to comment #21)
> Hi,
> 
> Sorry, didn't see that the legal issues have been put to bed on this on.
> 
> Happy to continue with this package, just let me know.
> 
> Regards,
> Jon

I need your FAS name in order to sponsor you. That's the last step hopefully.
Comment 23 Jon Kent 2013-03-22 09:31:56 EDT
Hi,

Great!! My FAS name is jondkent.

Regards,

Jon
Comment 24 Peter Lemenkov 2013-03-22 10:02:48 EDT
Unblocking FE-NEEDSPONSOR - I've just sponsored Jon.
Comment 25 Peter Lemenkov 2013-03-22 10:04:24 EDT
OK, I don't see any other issues so this package is

APPROVED.



ps proceed with the following now:

* https://fedoraproject.org/wiki/Package_SCM_admin_requests#New_Packages
Comment 26 Jon Kent 2013-03-22 11:53:04 EDT
New Package SCM Request
=======================
Package Name: ptpd
Short Description: ptpd implements the Precision Time Protocol as per IEEE 1588
Owners: jondkent
Branches: f18 f19 el6
InitialCC:
Comment 27 Peter Lemenkov 2013-03-22 12:07:40 EDT
Jon, you forgot to raise fedora-cvs flag to '?'.
Comment 28 Jon Kent 2013-03-22 12:29:17 EDT
Hi,

OK I know I'm being dumb, but how/where do you do that?
Comment 29 Peter Lemenkov 2013-03-22 12:44:37 EDT
(In reply to comment #28)
> Hi,
> 
> OK I know I'm being dumb, but how/where do you do that?

Heh, you're not the first who asked me :). Look at this screenshot:

* http://peter.fedorapeople.org/stuff/bzflags2.png
Comment 30 Peter Lemenkov 2013-03-22 12:46:11 EDT
(In reply to comment #28)
> Hi,
> 
> OK I know I'm being dumb, but how/where do you do that?

One note - please don't reset mine fedora-review flag - just raise another one (fedora-cvs)
Comment 31 Jon Kent 2013-03-22 12:54:45 EDT
Hi,

well that was obvious ;)

fedora-cvs flag set to ?

Cheers,
Jon
Comment 32 Jon Ciesla 2013-03-22 13:23:36 EDT
Git done (by process-git-requests).
Comment 33 Jon Kent 2013-03-22 17:51:30 EDT
Hi,

Thanks for that.

I'm getting:

* Permission denied (publickey)

when I run:

fedpkg clone ptpd
or
git clone ssh://jondkent@pkgs.fedoraproject.org/ptpd

is that expected?

Thanks,

Jon
Comment 34 Jon Kent 2013-03-22 17:56:04 EDT
Hi,

Please ignore my previous comment, this is now working

Thanks again
Comment 35 Peter Lemenkov 2013-03-23 00:59:09 EDT
*** Bug 556611 has been marked as a duplicate of this bug. ***
Comment 36 Jon Kent 2013-03-27 17:41:23 EDT
Hi,

OK, set this up in the repo.  Koji seems to like it, and I've changed it from using SysV init to systemd.  This is presently only in the devel tree, not placed anything in the f18/19 or el6 branches yet.

Whats the next step here is anything?

Thanks again,

Jon
Comment 37 Jon Stanley 2013-03-27 17:46:48 EDT
Next steps would be using fedpkg switch-branch and git cherry-pick (my workflow for if I want the same thing on multiple branches) and put it in the f18/f19 branches and build there. fedpkg is smart enough to know to build for the right tags if you do fedpkg build in a branch.

Next, you use fedpkg update in order to create a bodhi update, or the bodhi web interface.

However, note that RHEL already ships linuxptp-0-0.6.20121114gite6bbbb.el6.x86_64.rpm (it was part of 6.4) so I'm not sure if this is moot for el6 at this point or not.
Comment 38 Jon Kent 2013-03-29 17:54:06 EDT
Hi,

For some reason bodhi doesn't seem to want to play ball with f19.   F18 worked fine though via bodhi:

F18 request output:

[jon@jason ptpd (f18)]$ fedpkg update
Creating a new update for  ptpd-2.2.0-1.fc18 
Update successfully created
================================================================================
     ptpd-2.2.0-1.fc18
================================================================================
    Release: Fedora 18
     Status: pending
       Type: newpackage
      Karma: 0
    Request: testing
      Notes: First release of this pacakage
  Submitter: jondkent
  Submitted: 2013-03-29 21:49:07
   Comments: bodhi - 2013-03-29 21:49:10 (karma 0)
             This update has been submitted for testing by
             jondkent.

  https://admin.fedoraproject.org/updates/ptpd-2.2.0-1.fc18


F19 request output:

[jon@jason ptpd (f18)]$ fedpkg switch-branch f19
Switched to branch 'f19'
[jon@jason ptpd (f19)]$ fedpkg update
Creating a new update for  ptpd-2.2.0-1.fc19 
ptpd-2.2.0-1.fc19 not tagged as an update candidate
[jon@jason ptpd (f19)]$ 

Am I missing something here ?

Thanks,
Jon
Comment 39 Jon Kent 2013-04-05 15:54:26 EDT
Hi,

Chatted to a few people at Red Hat (UK) about the error I'm getting.

Heres what I got back:

"
This is a (known) bad error message. When I've come across it, it's meant that the build isn't tagged for fc19 because it failed during the mass rebuild or some other time in the update process.

That seems off since this package built in Koji [1] successfully. This is probably worth asking Luke Macken about since it's probably a bug.

1. http://koji.fedoraproject.org/koji/buildinfo?buildID=407119
"
Now I don't know Luke so I'm not going to reach out to him about this.  However, does this make sense?  I've tried again tonight but still get this error, so something doesn't like me ;)

Thanks,
Jon
Comment 40 Jon Stanley 2013-04-06 10:36:03 EDT
Actually it looks like that is tagged appropriately and in F19 now. You can tell because the build is in the koji tag 'f19' not 'f19-updates-candidate'

So there's nothing to do in bodhi here, it should just be in F19.
Comment 41 Jon Kent 2013-04-08 17:21:20 EDT
Hi,

Thanks for that.  Is there a way to clarify that at all as I can't figure out how to verify this.

Thanks,
Jon
Comment 42 Jon Kent 2013-04-11 17:45:39 EDT
Hi,

Are there any other actions required here?

Regards,
jon
Comment 43 Jon Stanley 2013-04-11 18:07:25 EDT
I don't see the automated bodhi comments in here for the stable branches. Did you not link the updates to this bug? (fine if not, I just always do)

As for figuring out the tagging situation, you can look at the koji link in comment #39 and see that the "tag" is listed as f19. If this were pending a bodhi update, it would be in the tag 'f19-updates-candidate' and bodhi would automatically move it as appropriate.

So long as the updates for the stable branches have made it to stable, I don't see anything further needing to be done here.
Comment 44 Jon Kent 2013-04-11 18:29:28 EDT
Hiya,

Thanks for that.

Ahhh, whoops, adding the bug number didn't occur to me - obvious really now I think about it.

FC18 has been tagged stable, FC19 I think is suck because of the alpha-freeze and it looks like I'd need to justify why a new package should be introduced.  EL6, as you mentioned, is probably not worth the effort (I might change my mind on that though).

So if thats all OK, then I'd agree there is probably not much left to do, thanks for all your help here.

Regards,
Jon
Comment 45 Peter Lemenkov 2013-04-11 23:47:47 EDT
Ok, time to close this - everything was build and pushed to stable branches.

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