Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

khal: update to 0.10.5. #38583

Merged
merged 1 commit into from Aug 13, 2022
Merged

Conversation

dataCobra
Copy link
Contributor

Testing the changes

  • I tested the changes in this PR: YES

Local build testing

  • I built this PR locally for my native architecture, (x86_64)
  • I built this PR locally for these architectures (if supported. mark crossbuilds):
    • aarch64-musl (crossbuild)

Since version 0.10.5 there is no longer a __khal file. This is why I deleted line 26.

@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 10, 2022

Sadly the last PR #32292 doesn't explain why the make_check=ci_skip is added.

What do I need to do for xlint to not fail?

EDIT: I removed the line for testing purposes and force-pushed.

@paper42
Copy link
Member

paper42 commented Aug 10, 2022

Failed: DID NOT RAISE <class 'khal.khalendar.exceptions.CouldNotCreateDbDir'>

It seems like it's trying to chmod and create some directories in a place where it shouldn't be possible which succeeds in CI because there, it runs as root.

What do I need to do for xlint to not fail?

Add a comment why it's happening - something like # some tests fail when running as root

@classabbyamp
Copy link
Member

CHANGE we are not shipping a zsh completion file anymore but provide documentation on how to generate completion files for bash, zsh, and fish (see the install section of the documentation)

Packagers: please generate and ship those completion files if possible

__khal was removed because it got replaced by this mechanism

also, please add the changelog: https://raw.githubusercontent.com/pimutils/khal/master/CHANGELOG.rst

cc maintainer @Anachron

@dataCobra
Copy link
Contributor Author

CHANGE we are not shipping a zsh completion file anymore but provide documentation on how to generate completion files for bash, zsh, and fish (see the install section of the documentation)
Packagers: please generate and ship those completion files if possible

__khal was removed because it got replaced by this mechanism

Oh, that's what I was looking for yesterday. Thank you for finding it! I read two changelogs
yesterday about 0.10.5 and none of these had this info. Because of this I just removed the
file.

I'll try to get this implemented.

also, please add the changelog: https://raw.githubusercontent.com/pimutils/khal/master/CHANGELOG.rst

Where do you want me to add the CHANGELOG.rst? -> usr/share/doc/?

I've also tried to create the manpage, but I couldn't really figure out how to do it the best way.

@classabbyamp
Copy link
Member

just changelog="url" in the template

@classabbyamp
Copy link
Member

https://github.com/pimutils/khal/blob/master/doc/source/install.rst#shell-completion

just add the commands that redirect to a file, then vcomplete those files in post_install

@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 11, 2022

https://github.com/pimutils/khal/blob/master/doc/source/install.rst#shell-completion

just add the commands that redirect to a file, then vcomplete those files in post_install

I assumed you mean that and I tried, but the options for khal that are autocompleted doesn't
even work... I've also installed directly into a virtualenv with pip to check if the behavior
is different. But it's the same. The completed options are not implemented in khal.

EDIT:
Here is an error message:

Error: Got unexpected extra argument (configure)

@dataCobra dataCobra force-pushed the khal-update-10.5 branch 2 times, most recently from e498185 to 04dc9a9 Compare August 11, 2022 05:29
@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 11, 2022

This software does need further testing. I don't know yet what is causing these problems.

I'll investigate further.

@Anachron
Copy link
Contributor

Anachron commented Aug 11, 2022

This looks fine to me, do you want me to test it or just wanted to notify me?

Edit: Ok just tell me when you need my help.

@classabbyamp
Copy link
Member

classabbyamp commented Aug 11, 2022

this seems to give completions files that work (tested zsh), and it doesn't require vendoring the files in filesdir

diff --git a/srcpkgs/khal/template b/srcpkgs/khal/template
index 6b74897878..2f6331ed31 100644
--- a/srcpkgs/khal/template
+++ b/srcpkgs/khal/template
@@ -3,10 +3,10 @@ pkgname=khal
 version=0.10.5
 revision=1
 build_style=python3-module
-hostmakedepends="python3-setuptools"
-depends="python3-setuptools python3-click python3-click-log python3-configobj
+hostmakedepends="python3-setuptools python3-click python3-click-log python3-configobj
  python3-dateutil python3-icalendar python3-pytz python3-tzlocal
  python3-urwid python3-xdg python3-atomicwrites"
+depends="$hostmakedepends"
 checkdepends="python3-pytest python3-freezegun vdirsyncer $depends"
 short_desc="Command-line calendar build around CalDAV"
 maintainer="Anachron <gith@cron.world>"
@@ -25,8 +25,9 @@ pre_build() {
 
 post_install() {
        vlicense COPYING
-       vcompletion "${FILESDIR}/khal.bash" bash
-       vcompletion "${FILESDIR}/khal.fish" fish
-       vcompletion "${FILESDIR}/khal.zsh" zsh
+       for sh in bash fish zsh; do
+               env PYTHONPATH=$DESTDIR/$py3_sitelib _KHAL_COMPLETE="${sh}_source" $DESTDIR/usr/bin/khal > "khal.${sh}"
+               vcompletion "khal.${sh}" $sh
+       done
        vsconf khal.conf.sample
 }

@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 11, 2022

Well, I should've used a testuser for this purpose and not my main user. The dotfiles on my
main user where the reason for the problems while testing...

Lesson learned. I'll use a testuser from now on.

EDIT: I've checked my bash aliases for possible khal stuff...

@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 11, 2022

Thanks a lot for the help. I've learned so much while updating this package.

@dataCobra
Copy link
Contributor Author

The next step is to get the creation of the manpage working for khal.

At the moment I've no idea how to accomplish that.

For that I'll create a new PR.

@classabbyamp
Copy link
Member

cd doc
make man

should do it, but you may need some more hostmakedepends for it to work

@dataCobra
Copy link
Contributor Author

I've tried to add this in, but Sphinx can't find the khal module when it tries to import.

diff --git a/srcpkgs/khal/template b/srcpkgs/khal/template
index 2f6331ed31..f06be57363 100644
--- a/srcpkgs/khal/template
+++ b/srcpkgs/khal/template
@@ -3,10 +3,13 @@ pkgname=khal
 version=0.10.5
 revision=1
 build_style=python3-module
-hostmakedepends="python3-setuptools python3-click python3-click-log python3-configobj
- python3-dateutil python3-icalendar python3-pytz python3-tzlocal
- python3-urwid python3-xdg python3-atomicwrites"
-depends="$hostmakedepends"
+hostmakedepends="python3-setuptools python3-Sphinx python3-sphinxcontrib
+ python3-click python3-click-log python3-configobj python3-dateutil
+ python3-icalendar python3-pytz python3-tzlocal python3-urwid python3-xdg
+ python3-atomicwrites"
+depends="python3-click python3-click-log python3-configobj python3-dateutil
+ python3-icalendar python3-pytz python3-tzlocal python3-urwid python3-xdg
+ python3-atomicwrites"
 checkdepends="python3-pytest python3-freezegun vdirsyncer $depends"
 short_desc="Command-line calendar build around CalDAV"
 maintainer="Anachron <gith@cron.world>"
@@ -30,4 +33,6 @@ post_install() {
 		vcompletion "khal.${sh}" $sh
 	done
 	vsconf khal.conf.sample
+        cd doc/
+        make man
 }

The following error message is stopping the process:

sphinx-build -b man -d build/doctrees   source build/man
Running Sphinx v5.0.2

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/sphinx/config.py", line 343, in eval_config_file
    exec(code, namespace)
  File "/builddir/khal-0.10.5/doc/source/conf.py", line 17, in <module>
    import khal
ModuleNotFoundError: No module named 'khal'

make: *** [Makefile:131: man] Error 2
=> ERROR: khal-0.10.5_1: post_install: 'make man' exited with 2
=> ERROR:   in post_install() at srcpkgs/khal/template:37

@dataCobra
Copy link
Contributor Author

dataCobra commented Aug 11, 2022

I got it working!

But it has a dependency which is not yet available in remote repositories.

Should I add the package to this PR or create a new PR?
PKG: python3-sphinxcontrib-newsfeed

EDIT: formatting

@Anachron
Copy link
Contributor

Did you add PYTHONPATH as in Arch Linuxs https://github.com/archlinux/svntogit-community/blob/packages/khal/trunk/PKGBUILD#L31?

@dataCobra
Copy link
Contributor Author

I've created a separate PR #38602 for the new host make dependency now.

I've successful tested the building process with khal and python3-sphinxcontrib-newsfeed.

I also use the packages now already on my host machine.

@paper42
Copy link
Member

paper42 commented Aug 11, 2022

add that commit here, it doesn't make sense separately from this PR and this PR doesn't work without it

@dataCobra
Copy link
Contributor Author

add that commit here, it doesn't make sense separately from this PR and this PR doesn't work without it

I've done it as you said. :)

@dataCobra
Copy link
Contributor Author

Hey @Anachron,

does this still look fine to you? 🙂

@Anachron
Copy link
Contributor

Yes @dataCobra , excellent work!

@dataCobra
Copy link
Contributor Author

Ups, I did git magic and messed up my branch...

I'll fix it and send it again.

@dataCobra
Copy link
Contributor Author

Alright, everything is again as it should be. 👍

@dataCobra
Copy link
Contributor Author

If something is missing to get it merged, just let me know. 🙂

@dataCobra dataCobra force-pushed the khal-update-10.5 branch 2 times, most recently from 695f2bb to 8f65bff Compare August 12, 2022 12:10
@dataCobra
Copy link
Contributor Author

It looks like the sphinxcontrib-newsfeed package does use a testing mechanism.

@classabbyamp
Copy link
Member

if there is, it does not have any indication of it in the repo

@paper42
Copy link
Member

paper42 commented Aug 12, 2022

It only checks if all required packages are installed and fails because Sphinx is not installed, add it to $checkdepends with checkdepends="$depends" .

@dataCobra
Copy link
Contributor Author

Hey @paper42,

It only checks if all required packages are installed and fails because Sphinx is not installed, add it to $checkdepends with checkdepends="$depends" .

I've added Sphinx, but it still fails because of a TypeError. I've tried to get rid of it
but was not able to find a solution.

@classabbyamp
Copy link
Member

classabbyamp commented Aug 12, 2022

even when adding the test_suite argument to setup.py, setup.py test fails, probably because there are no tests to find and run.

fwiw, sphinx doesn't even need sphinxcontrib-newsfeed for generating the manpage. with this patch and removal of sphinx_rtd_theme (which is just for the html docs) from the hostmakedepends it still generates the same manpages. There are warnings but they don't affect the output.

--- a/doc/source/conf.py
+++ b/doc/source/conf.py
@@ -107,7 +107,6 @@
     'sphinx.ext.autodoc',
     'sphinx.ext.intersphinx',
     'sphinx.ext.todo',
-    'sphinxcontrib.newsfeed',
 ]
 
 # Add any paths that contain templates here, relative to this directory.

@dataCobra
Copy link
Contributor Author

I've added the patch and and changed the khal template accordingly. I've also removed the sphinx_rtd_theme dependency. After that I successful build the package.

The push does no longer contain the python3-sphinxcontrib-newsfeed package.

@classabbyamp classabbyamp merged commit f8b18a5 into void-linux:master Aug 13, 2022
@dataCobra dataCobra deleted the khal-update-10.5 branch August 13, 2022 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants