Bug 1714432

There are some tags; is there any reason not to be packaging one of those?

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

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.

(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

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

(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

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.

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.

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.

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.

github.com/robfig/cron/v3 won't work as we don't have modules enabled in Fedora yet.

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

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.