-
Notifications
You must be signed in to change notification settings - Fork 79
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
ci: use Nix shell and upgrade Qt to 5.15.8 #9232
Conversation
Jenkins BuildsClick to see older builds (159)
|
3afb26e
to
658d914
Compare
Now that's something I've never seen before:
|
closing, but branch should remain |
0579a8e
to
463b470
Compare
e4e8851
to
f16eea8
Compare
I can't help but notice that you're doing a lot of work here without documenting anything in the issue. |
@jakubgs, true. Let me update the issue. |
@anastasiyaig, can you please help with testing of the build? |
@yakimant i am really sorry i totally forgot about that. Could you rebase and i will give it a spin? |
# Qt 5.15.8 copy from 76973ae3b30a88ea415f27ff53809ab8f452e2ec | ||
# Edited: | ||
# - temporary break Darwin support | ||
# - remove unsupported testers, env., config.allowAliases | ||
# - mkDerivation without finalAttrs | ||
# - change fetch* parameter from hash to sha256, rmove fetchLFS | ||
# - fix makeSetupHook | ||
# - switch from makeScopeWithSplicing back to makeScope | ||
# See diff for a full list of changes |
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 worry that as we keep adding changes to this copy of qt
derivation we'll end up in a situation where we can't use the qt
available in nixpkgs
due to these differences. It's good that you commented on the changes here, but it would be difficult to compare the changes.
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.
Currently it these 2 commits:
❯ git log nix/pkgs/qt-5/
commit 77c282f51cd0cd286c0a0a5eb00b1be8855eaeea
Author: Anton Iakimov <[email protected]>
Date: Wed Apr 10 13:50:08 2024 +0200
ci: fix qtwebengine issue
commit b4075c62d2afadec5523a6a9634510badd65e7c1
Author: Anton Iakimov <[email protected]>
Date: Fri Jan 19 16:23:26 2024 +0100
ci: ammend qt derivation to run on 20.09
commit d17d77ae066c46284e7e0d90ed31213e330e3eb4
Author: Anton Iakimov <[email protected]>
Date: Fri Jan 19 13:30:40 2024 +0100
ci: copy qt 5.15.8 package
Thcecking the diffs you can see, what exactly was changed.
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 is looking pretty good. But I don't see a CI Nix build in the checks.
You need to add a job here:
https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/
You can copy the linux
one, just update the Jenkinsfile
path and context name so it appears separately in GH checks.
You can stop such a job from running for other PRs by using the Filter by name(with wildcards)
setting in Branch Sources
:
Just set exclude
to *
and include
to PR-9232
(or something like that).
a7211c9
to
1fc2e31
Compare
ci/Jenkinsfile.linux-nix
Outdated
|
||
stage('Package') { | ||
steps { script { | ||
linux.bundle('--debug=b tgz-linux', 1, true) |
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.
Please use named arguments, positional arguments obscure what is happening.
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.
Oh, I need to modify linux.bundle
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.
Actually, let's keep it for later.
Because it didn't have positional arguments originally.
Will make a note for leftovers.
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.
You shouldn't have to change anything, you should be able to just use them as is.
scripts/init_app_dir.sh
Outdated
mkdir -p "${APP_DIR}/usr/bin" | ||
mkdir -p "${APP_DIR}/usr/lib" | ||
mkdir -p "${APP_DIR}/usr/qml" | ||
mkdir -p "${APP_DIR}/usr/i18n" | ||
mkdir -p "${APP_DIR}/usr/bin/StatusQ" | ||
mkdir -p "${APP_DIR}/usr/plugins/platforminputcontexts/" |
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.
If all operations are performed in ${APP_DIR}
why can't we just cd
into it instead of adding ${APP_DIR}
to every single path?
Also, all these can be done with a single mkdir
.
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.
Decided to keep as is, without cd
.
If I cd and mkdir, need to cd back.
If I continue in that dir will need to ammend copy from paths.
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.
Not sure why would you need to cd
to mkdir
, but it's not a big deal.
Also, in scripts pushd
/popd
is more common for similar reasons.
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.
@jakubgs, looks like it's because you are an author of PR. |
DEATH TO DOCKER, LONG LIVE NIX
EDIT: This alone will not work since we'd need some kind of solution for GLibc version issue on older systems.