-
Notifications
You must be signed in to change notification settings - Fork 2k
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
app-misc/anki: add 23.12.1 #31122
app-misc/anki: add 23.12.1 #31122
Conversation
Pull Request assignmentSubmitter: @antecrescent app-misc/anki: At least one of the listed packages is maintained entirely by non-GitHub developers! Linked bugsIn order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
Pull request CI reportReport generated at: 2023-05-22 04:03 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-05-22 04:18 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-05-24 21:49 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-06-07 20:49 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-06-12 08:41 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-06-18 21:00 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-08-03 01:55 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-08-27 19:51 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
0d4505e
to
6671f25
Compare
Pull request CI reportReport generated at: 2023-08-27 20:45 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-08-27 21:00 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
app-misc/anki/anki-2.1.66.ebuild
Outdated
pkg_setup() { | ||
python-single-r1_pkg_setup | ||
# The Python and Yarn environment variables prevent a download of the binaries | ||
export PYTHON_BINARY=/bin/false YARN_BINARY=/bin/false \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to specify hard paths here? These assumptions might not be applicable in eprefix and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, absolute paths are required. I did not yet find out why, or how to change that.
The binaries provided by *_BINARY
get used during compilation and need to be exported during src_{compile,test}
. I'm not sure if that was evident.
According to @thesamesam, they need to be prefixed with ${BROOT}
:
[01:01] colores: I'm currently implementing changes for a review I got on PR 31122. Do I need to worry about EPREFIX or ROOT when providing the absolute path to a binary? The binary is linked into the build
system during src_compile/test.
[01:01] @ sam_: is it linked into the final binary or no?
[01:01] @ sam_: (what is it for)
[01:06] colores: Sorry, build *dir*. The the symlinked binaries (protoc, node, cargo_nextest) are used during compilation and do not get linked into the final binary.
[01:07] @ sam_: ah, BROOT indeed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's okay to rely on PATH lookups for things like false
, but not so much for things in BDEPEND
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in fa3d721
app-misc/anki/anki-2.1.66.ebuild
Outdated
LICENSE="AGPL-3+ Apache-2.0 BSD CC-BY-3.0 GPL-3+ MIT" | ||
|
||
SLOT="0" | ||
KEYWORDS="~amd64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DroppedKeywords: version 2.1.66: x86
dropped on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unfortunately, I don't have a dedicated testing environment yet and I don't own an x86 machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my understanding you still keep keywords on upgrades even if you can't test it. You would only drop them if you had to add dependencies which weren't keyworded.
https://devmanual.gentoo.org/keywording/index.html#keywording-on-upgrades
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readded keyword ~x86
in 1dd818e
app-misc/anki/anki-2.1.66.ebuild
Outdated
# PyQt6: Actually need bindings for QtGui with PyQt_OpenGL enabled, | ||
# that is qtbase[opengl,gui] and PyQt6[gui], not PyQt6[gui,opengl]. | ||
# dev-qt/qtgui:5 enables opengl support by default. | ||
# If !qt6 block PyQt6, because of the Qt6 detection mechanism |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would patching the detection be viable? The apparent automagic for detecting qt is quite unfortunate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should do it: fa3d721
app-misc/anki/anki-2.1.66.ebuild
Outdated
eapply_user | ||
} | ||
|
||
src_compile() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add filter-lto, as you have to use the artisan build system from anki you miss out the cargo eclass doing it for you.
https://devmanual.gentoo.org/eclass-reference/flag-o-matic.eclass/index.html
= note: some extern
functions couldn't be found; some native libraries may need to be installed or have their path specified
= note: use the -l
flag to specify native libraries to link
= note: use the cargo:rustc-link-lib
directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)
error: could not compile runner
(bin "runner") due to previous error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request CI reportReport generated at: 2023-08-29 10:00 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
ping. I implemented the requested changes last week. Is there anything else I can do apart from squashing and rebasing? |
This comment was marked as resolved.
This comment was marked as resolved.
Pull request CI reportReport generated at: 2023-11-26 18:11 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Thanks for the ping earlier - I'm swamped atm, but I agree this needs doing. Can you remind me in about two weeks please? Do really appreciate the patience. |
Pull request CI reportReport generated at: 2023-11-27 18:41 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Changes to last iteration:
@thesamesam Your reminder, as requested! Not sure if you read my dm on IRC last weekend, so I thought I'd ping you publicly just in case :P (Thanks a lot for reviewing this rather massive ebuild.) |
Pull request CI reportReport generated at: 2023-12-13 13:47 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Pull request CI reportReport generated at: 2023-12-13 14:17 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
@antecrescent hey there, good work.
I am also thinking of adding a systemd/initrd file for the sync server. |
Closes: https://bugs.gentoo.org/914774 Closes: https://bugs.gentoo.org/911923 Closes: https://bugs.gentoo.org/782373 Signed-off-by: Lucio Sauer <watermanpaint@posteo.net>
Changes to last iteration:
|
Pull request CI reportReport generated at: 2024-01-09 03:54 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
mkdir -p out/pyenv/bin || die | ||
ln -s "${PYTHON}" out/pyenv/bin/"${EPYTHON}" || die | ||
ln -s "${EPYTHON}" out/pyenv/bin/python || die | ||
# TODO: ln -s "${BROOT}/usr/bin/protoc-gen-mypy" out/pyenv/bin || die |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Atm, pseudo-venv.patch removes the code that generates stubs for Anki's protobuf files. I figured that might be ok for now, since it's "only" type hints that are missing.
I have an ebuild for mypy-protobuf on my disk but I struggle to set up the tests correctly, so I haven't published it yet.
Thank you for the review and the suggestions!
Unfortunately, the link is broken. I assume you meant to link to your PR?
https://github.com/ankitects/anki/blob/1a1d4d5419c6b57ef3baf99c9d2d9cf85d36ae0a/build/ninja_gen/src/build.rs#L357-L363
Can you explain what you mean by masking, please? Maybe it doesn't apply anymore since the 23.12.1 ebuild is quite different from the 23.10.1 one.
AFAIK
Done.
Why? Upstream recommends that you build and install the wheels into your local venv. Also, from what I understand, pip uses dist-info metadata from wheels to resolve dependencies (see: https://bugs.gentoo.org/920330).
If you install the GUI, the sync server is baked into the rsbridge backend. You can even call it directly, so
I don't know yet, what's going on here.
Looking at your ebuild, why did you choose not to install dist-info?
I don't understand this.
That's a good idea 👍 |
Signed-off-by: Lucio Sauer <watermanpaint@posteo.net>
Pull request CI reportReport generated at: 2024-01-09 06:04 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
- Reword pseudo-venv.patch for clarity. Signed-off-by: Lucio Sauer <watermanpaint@posteo.net>
Pull request CI reportReport generated at: 2024-01-09 13:34 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Signed-off-by: Lucio Sauer <watermanpaint@posteo.net>
Pull request CI reportReport generated at: 2024-01-09 23:59 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
I took great inspiration from NixOS' Anki package. I'd be happy about some feedback!
Should I copy the
gl-fixup
patch from NixOS? I've had no problems with the GL library so far.I'll gladly co-maintain this package if need be.
TODO:
bzip2_sys
appears to compile the library statically, why?The crate was dropped with version 2.1.66.
Isdev-libs/openssl
needed?The OpenSSL library is linked against _rsbridge.so only when testing, otherwise it uses Rustls.