Page MenuHomeFreeBSD

Updating py-khal to v0.9.8
ClosedPublic

Authored by rigoletto on Dec 10 2017, 1:46 PM.

Details

Summary
  • update deskutils/py-khal to 0.9.8 version
  • switch it to DISTVERSION
  • some minor alphabetizing
  • add new dependencies
  • added an OPTION to setproctile

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rigoletto created this revision.Dec 10 2017, 1:46 PM
tcberner added inline comments.Dec 10 2017, 2:35 PM
deskutils/py-khal/Makefile
29 ↗(On Diff #36421)

^ I think the lowest version at the moment is 3.4 -- so why the change?

rigoletto updated this revision to Diff 36422.Dec 10 2017, 2:46 PM

Yes, this patch started for a older version. I forgot to update the minimum python version to python:3.4+.

Thanks!

rigoletto marked an inline comment as done.Dec 10 2017, 2:46 PM

Done.

tcberner added inline comments.Dec 10 2017, 3:05 PM
deskutils/py-khal/Makefile
1 ↗(On Diff #36422)

^unless this is you, or you have the permission, you are not allowed to remove the "created by" line :)

I am out now and so I can't look for, but shouldn't the port header be cleanased and leave only the "# $freebsd$" line?

Doesn't that include removing that "created by" line?

Thanks!

mat added inline comments.Dec 11 2017, 1:59 PM
deskutils/py-khal/Makefile
35 ↗(On Diff #36422)

Empty, remove.

rigoletto updated this revision to Diff 36457.Dec 11 2017, 2:04 PM
  • bring "created by" back
  • remove the empty OPTIONS_DEFAULT
rigoletto marked 2 inline comments as done.Dec 11 2017, 2:04 PM

Done.

github_lostpackets.de added inline comments.
deskutils/py-khal/Makefile
47 ↗(On Diff #36457)

I would be really surprised if this worked.
I believe this needs a patch like this: https://github.com/pimutils/khal/commit/054061221f9726107ebc59a51f944f15bf0afd7b (which currently breaks for unknown reasons on python-3.7-dev

rigoletto added inline comments.Dec 13 2017, 1:47 AM
deskutils/py-khal/Makefile
47 ↗(On Diff #36457)

In short you mean this also needs "devel/py-pytest" and "devel/py-pytest-runner" for v0.9.8?

rigoletto updated this revision to Diff 36923.Dec 22 2017, 10:21 PM

Updating with the suggested modifications, however still fail to test:

https://pastebin.com/C4mKajnx

Thanks!

rigoletto updated this revision to Diff 36924.Dec 22 2017, 10:27 PM
rigoletto marked 2 inline comments as done.

Minor.

rigoletto updated this revision to Diff 36990.Dec 25 2017, 3:35 PM
This comment was removed by rigoletto.
rigoletto updated this revision to Diff 36991.Dec 25 2017, 3:44 PM

Please disregard the previous diff, that was mistakenly added in here.

Sorry.

There are two different kind of errors here:

  1. the filesystem of the build service doesn't seem to support mtime of files, while we could mock this, it's kind of the point to test if an updated file is caught my khal. Do you know what kind of system the build is run on?
  2. the locale seems to be not/misconfigured, exporting the correct locale settings should fix those errors, something like exporting LC_ALL=en_US.utf-8 and LANG=en_US.utf-8 should do the tick ( see http://click.pocoo.org/5/python3/#python-3-surrogate-handling )

As the first issue is not easily fixable, I would suggest either patching the tests to make them xfail or not run them for now.

deskutils/py-khal/Makefile
47 ↗(On Diff #36457)

The new setup.cfg file is also needed (if FreeBSD doesn't have anything in place to autodiscover that setting which I don't know about).

rigoletto updated this revision to Diff 37143.Dec 28 2017, 3:57 PM

Removing TEST. The inclusion was making the things too cumbersome.

Thanks!

rigoletto added a comment.EditedDec 28 2017, 4:34 PM

There are two different kind of errors here:

  1. the filesystem of the build service doesn't seem to support mtime of files, while we could mock this, it's kind of the point to test if an updated file is caught my khal. Do you know what kind of system the build is run on?
  2. the locale seems to be not/misconfigured, exporting the correct locale settings should fix those errors, something like exporting LC_ALL=en_US.utf-8 and LANG=en_US.utf-8 should do the tick ( see http://click.pocoo.org/5/python3/#python-3-surrogate-handling )

As the first issue is not easily fixable, I would suggest either patching the tests to make them xfail or not run them for now.

  • FreeBSD workstation.privacychain.ch 11.1-RELEASE-p6 FreeBSD 11.1-RELEASE-p6 #0 r326729: Sat Dec 9 13:58:14 -02 2017 root@workstation.privacychain.ch:/usr/obj/usr/src/sys/GENERIC amd64
  • Locale: LANG=en_US.UTF-8 LC_CTYPE=en_US.UTF-8 LC_COLLATE=C LC_TIME=en_US.UTF-8 LC_NUMERIC=en_US.UTF-8 LC_MONETARY=pt_BR.UTF-8 LC_MESSAGES=en_US.UTF-8 LC_ALL=

I also tried with LC_ALL="en_US.UTF-8", same thing as far I remember.

Thanks!

I'll have a look at this later, I'll keep track of this in khal's issue tracker (see https://github.com/pimutils/khal/issues/745 ).

This revision is now accepted and ready to land.Dec 28 2017, 11:28 PM
adridg requested changes to this revision.Dec 29 2017, 2:35 PM

Please double-check upstream's comment about setup.cfg.

deskutils/py-khal/Makefile
47 ↗(On Diff #36457)

I recall there being a new file to install, some sample config file. It looks like that got lost when dropping the do-test target.

This revision now requires changes to proceed.Dec 29 2017, 2:35 PM

Please double-check upstream's comment about setup.cfg.

It's not needed when the tests are not run via the ports framework (which doesn't work atm, see https://github.com/pimutils/khal/issues/745 ).

Can it be merged already? :-)

Thanks!

mat added a comment.Jan 11 2018, 11:52 PM
In D13432#290787, @lbdm_privacychain.ch wrote:

Can it be merged already? :-)
Thanks!

This is a code review tool, not a bug tracker, people opening code reviews are usually responsible for committing the changes. If you are not a committer, using the code review is good, but you should probably open a PR in the end with the final patch so that it gets in the queue.

rigoletto added a comment.EditedJan 12 2018, 12:40 AM

There is one already.

Thanks!

adridg accepted this revision.Jan 12 2018, 9:47 PM
adridg added a subscriber: rakuco.

OK by me, but I'd still need an OK from @tcberner or @rakuco to commit it (or they could do it themselves).

This revision is now accepted and ready to land.Jan 12 2018, 9:47 PM

Built and installed naively it refuses to run until I set LANG to a UTF-8 locale (e.g. en_US.UTF-8), but that's more a Python 3.6 / Click problem than khal.

rigoletto marked 2 inline comments as done.Jan 12 2018, 11:30 PM

I am marking those are "Done" since the subject was already discussed.

Will wait @github_lostpackets.de say something about the last issues pointed by @adridg, and if necessary add in a pkg-message or what be more correct.

Thanks!

rigoletto updated this revision to Diff 37947.Jan 14 2018, 8:13 PM

Add in pkg-message file.

This revision now requires review to proceed.Jan 14 2018, 8:13 PM
tcberner added inline comments.Jan 14 2018, 8:17 PM
deskutils/py-khal/Makefile
10 ↗(On Diff #37947)

^ @lbdm_privacychain.ch how about taking maintainership?

deskutils/py-khal/pkg-message
3 ↗(On Diff #37947)

only during run or also during build?

rigoletto updated this revision to Diff 37949.Jan 14 2018, 8:29 PM

Take maintainership and slightly modify pkg-message.

rigoletto marked 2 inline comments as done.Jan 14 2018, 8:31 PM

Done.

tcberner accepted this revision.Jan 14 2018, 8:31 PM

looks good to me.

This revision is now accepted and ready to land.Jan 14 2018, 8:31 PM
This revision was automatically updated to reflect the committed changes.