[Bug 1714432] New: Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

Bug ID: 1714432
           Summary: Review Request: golang-github-robfig-cron - Cron
                    library for go
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://mgoodwin.fedorapeople.org/golang/cron/golang-github-robfig-cron.spec
SRPM URL:
https://mgoodwin.fedorapeople.org/golang/cron/golang-github-robfig-cron-3-0....
Description: Cron library for go
Fedora Account System Username: mgoodwin

Scratch rawhide koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=35103082

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

Elliott Sales de Andrade  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Elliott Sales de Andrade  ---
There are some tags; is there any reason not to be packaging one of those?

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #2 from Mark Goodwin  ---
Hi Elliot,

the v3.0.0-rc1 tag is not released (rc1). As per the README.md:

It is currently IN DEVELOPMENT and will be considered released once a 3.0
version is tagged. It is backwards INCOMPATIBLE with both the v1 and v2
branches.

So I decided to package the HEAD commit version (and will update to the v3.0.0
version once it is tagged).

This is for grafana-6.2.x and later, which needs v3 (v1 and v2 are
incompatible).

Regards and thanks

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #3 from Elliott Sales de Andrade  ---
Yes, but the v3.0.0-rc1 tag is the same as the HEAD of the v3 branch, so it'd
be the same code, just with a proper version.

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #4 from Mark Goodwin  ---
(In reply to Elliott Sales de Andrade from comment #3)
> Yes, but the v3.0.0-rc1 tag is the same as the HEAD of the v3 branch, so
> it'd be the same code, just with a proper version.

grafana has vendored HEAD of the master branch, which is different (newer) to
HEAD of the v3 branch.
That leads me to think final tagged v3.0.0 (when it happens) will be same (or
close) to current master, and hence why grafana went with that rather than
current v3.

Regards

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #5 from Elliott Sales de Andrade  ---
I tried to run fedora-review, but the tests fail:

2019/05/29 17:25:40 cron: panic running job: YOLO
goroutine 23 [running]:
github.com/robfig/cron.(*Cron).runWithRecovery.func1(0xc0000b8410)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:161
+0xa0
panic(0x5574fb915300, 0x5574fb949740)
        /usr/lib/golang/src/runtime/panic.go:522 +0x1b9
github.com/robfig/cron.TestFuncPanicRecovery.func1()

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron_test.go:19
+0x3b
github.com/robfig/cron.FuncJob.Run(0x5574fb948ed8)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:92
+0x27
github.com/robfig/cron.(*Cron).runWithRecovery(0xc0000b8410, 0x5574fb94c0e0,
0x5574fb948ed8)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:165
+0x59
created by github.com/robfig/cron.(*Cron).run

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:199
+0x74b
2019/05/29 17:25:41 cron: panic running job: YOLO
goroutine 24 [running]:
github.com/robfig/cron.(*Cron).runWithRecovery.func1(0xc0000f4000)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:161
+0xa0
panic(0x5574fb915300, 0x5574fb949770)
        /usr/lib/golang/src/runtime/panic.go:522 +0x1b9
github.com/robfig/cron.DummyJob.Run(...)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron_test.go:30
github.com/robfig/cron.(*Cron).runWithRecovery(0xc0000f4000, 0x5574fb94c0c0,
0x5574fba4a5c0)

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:165
+0x59
created by github.com/robfig/cron.(*Cron).run

/builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/src/github.com/robfig/cron/cron.go:199
+0x74b
--- FAIL: TestNonLocalTimezone (2.02s)
    cron_test.go:261: expected job fires 2 times
FAIL
exit status 1
FAIL    github.com/robfig/cron  25.439s

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #6 from Mark Goodwin  ---
(In reply to Elliott Sales de Andrade from comment #5)
> I tried to run fedora-review, but the tests fail:

[...]
> /builddir/build/BUILD/cron-b41be1df696709bb6395fe435af20370037c0b4c/_build/
> src/github.com/robfig/cron/cron.go:199 +0x74b
> --- FAIL: TestNonLocalTimezone (2.02s)
>     cron_test.go:261: expected job fires 2 times
> FAIL
> exit status 1
> FAIL    github.com/robfig/cron  25.439s

Hi Elliott, thanks once again.

I ran fedora-review three times - the first failed (as you saw too), but then
2nd and 3rd runs passed.
It seems the TestNonLocalTimezone / cron_test.go may be racy (in terms of how
many times the job fires).

I'll open an upstream issue about that. In the mean-time, I'll put this review
request on hold - may be best to do anyway until v3.0.0 is tagged. Grafana can
continue to vendor github.com/robfig/cron until then.

Regards
-- Mark

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #7 from Elliott Sales de Andrade  ---
If I've read the test correctly, it looks like it schedules a job for 1 second
and 2 seconds in the future (in absolute time), waits 2 seconds and checks it's
been called twice. But given a bit of jitter, this is probably a bit fragile.

Either 2 seconds is not long enough to wait (because the initial setup took a
little while so the second fire is not exactly 2 seconds later, but a smidge
longer), or the first job didn't fire (because Now() was calculated just a bit
before a rollover to the next second.) Or possibly both of these.

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

Robert-André Mauchin  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from Robert-André Mauchin  ---
Packaged as golang-gopkg-robfig-cron-3 already. Maybe you could update it to
package the tip of the v3 branch?
Are you sure Grafana can't work with the v3 version? If it builds, it should be
ok.

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #9 from Mark Goodwin  ---
I posted issue #205 https://github.com/robfig/cron/issues/205 (based on
Elliott's Comment #7) which has now been fixed along with a bunch of other
issues; and he has now released and tagged v3.0.0. So I'll update
golang-gopkg-robfig-cron-3 to that release rather than the tip of v3 branch
(probably the same anyway), and recheck grafana.

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #10 from Mark Goodwin  ---
zebob, the v3.0.0 tagged release uses golang modules, which has resulted in
issues like https://github.com/robfig/cron/issues/209

So the import path would have to be github.com/robfig/cron/v3 (and
gopkg.in/robfig/cron.v3 will no longer work). I'm not sure what this kind of
thing means for the macros work just merged into rawhide :P but for now I think
we should keep golang-gopkg-robfig-cron-3 as it currently is.

[Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #11 from Robert-André Mauchin  ---
github.com/robfig/cron/v3 won't work as we don't have modules enabled in Fedora
yet.

needinfo canceled: [Bug 1714432] Review Request: golang-github-robfig-cron - Cron library for go

Product: Fedora
Version: rawhide
Component: Package Review

Package Review  has canceled Package
Review 's request for Mark Goodwin
's needinfo:
Bug 1714432: Review Request: golang-github-robfig-cron - Cron library for go
https://bugzilla.redhat.com/show_bug.cgi?id=1714432

--- Comment #13 from Package Review 
---
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.