sys-auth/polkit: bump to 0.119 and apply duktape patchset #1263
Conversation
|
Can you follow #863 (review) to make sure we can identify the downstream changes and also know that previous downstream changes are preserved? |
@pothos What constitutes a "downstream patch", exactly? To be honest with you, I have no idea what makes 5c4d184 a commit that needs to be forward-ported and I would appreciate any insight you can provide on this. |
|
"Downstream patches" are changes that were applied to the ebuild in this repository compared to the version in https://github.com/gentoo/gentoo (the upstream). Many of the "downstream" changes are things like in 5c4d184: installing Flatcar specific configuration files or files in Flatcar specific locations (keeping in mind that: /usr is read-only and gets auto-updated). |
|
The commits in The example commit I took is quite interesting because it relates to how Flatcar is updated: What is auto-updated is the |
|
Exciting to see the duktape-based polkit! 👍 Ideally, we should move the native Gentoo ebuilds, i.e. without Flatcar patches, to portage-stable. |
|
Getting the list easily requires that people follow this best practice but it wasn't done in the case of this package here. But if you update by resetting the package to upstream state and then adding the downstream changes in a separate commit (one big or many small as you like), it will be easy for the next person to get the list of downstream patches. |
|
Or actually this package here followed this practice somehow already and you can consider all commits from fb5590c onwards as downstream changes but the patches that are included in the new polkit release can be dropped. |
|
With my python2-eradicator hat on, I'd love this to get this in, as spidermonkey is still using python2. |
wrl
changed the title
wrl
force-pushed
the
wrl/polkit-0.119
branch
2 times, most recently
from
c5e253a
to
5c0b98b
Compare
|
The second commit should say re-apply downstream patches and also include the duktape patch as it is a downstream patch compared to Gentoo. |
|
CI is OK with arm64 but not with amd64, polkit pulls python through an intermediate dependency. 🤔 |
|
Looks like we'll have to disable |
Not sure if that's an option, grepping for sys-auth/polkit gives me among other lines: I'll need to check if realmd really needs introspection stuff from polkit. |
|
I had a look at realmd's and polkit build systemd and I think that realmd's dependency on polkit with introspection is bogus - it should be fine with being just |
|
And dropping introspection from polkit would be nice - it was giving me a headache when I tried to update gobject-introspection, because polkit then was trying to put or get some introspection things from |
|
@pothos Considering that the duktape patch is pulled from Gitlab and is therefore version-dependent, does it make sense to have it be part of the roll-up patch? It seems like that's just adding complexity next time we have to bump the version (i.e. "reformat these patches for the new version, but also re-fetch this other patch from Gitlab".) |
|
Yes, a separate commit would be nice which documents how to create it. |
|
Have updated this PR with separate duktape patchset commit. Note: this will not build with only the first commit ("bump to 0.119") applied because it depends on a version of Spidermonkey which we do not have in portage-stable. |
wrl
changed the title
sys-auth/realmd/realmd-0.17.0.ebuild
Outdated
| # flatcar changes: remove `[introspection]` from `sys-auth/polkit` | ||
| # it's the only component preventing us to build polkit without `introspection` | ||
| # support. |
There was a problem hiding this comment.
You could drop this comment - this package is our own, not based on anything from gentoo.
There was a problem hiding this comment.
Oh good to know. Any reason to not push this ebuild to ::gentoo then ?
EDIT: comment removed and commit reworded in the rebasing of the branch onto main.
There was a problem hiding this comment.
Oh good to know. Any reason to not push this ebuild to
::gentoothen ?
Probably no reason other than "no one got around to do it yet". :) I never contributed anything to gentoo, so I'd need to invest some time to see how it's done.
There was a problem hiding this comment.
Maybe we could first drop a comment into this issue: https://bugs.gentoo.org/486230 to see what's going on and mentioning that ::coreos-overlay holds an ebuild without so much Flatcar customization (except maybe for the tmpfiles).
Otherwise, I'm used to push ebuilds on ::guru1 when it's not really needed by ::gentoo and often ebuilds from ::guru finally land on ::gentoo.
Footnotes
There was a problem hiding this comment.
Sure, adding the comment and the ebuild (probably changing tmpfiles to keepdir or something) to guru sound good as first steps. I can add it to my TODO list, if you won't be able to do it. That would still mean that we would need to maintain our own fork of the ebuild, so it probably won't be a big win.
There was a problem hiding this comment.
Oh, and changing DBus policy path from /etc/dbus-1/system.d to /usr/share/dbus-1/system.d is also a flatcarism.
tormath1
force-pushed
the
wrl/polkit-0.119
branch
from
ddee956
to
59c6fa1
Compare
|
build fails for both boards:
but we can clearly see that Now comes the fun part, let's compare binaries between this PR and regular binaries: This PR, we can clearly see that there is no comparing with current output from a nightly: so the binaries look good but we still rely on the there is I'm tempted to remove |
|
@tormath1: |
|
@tormath1: I suppose that the problem is that |
|
@tormath1: Actually, gentoo upstream dropped |
I missed that 🤦 haha - I just don't understand from where it comes, it seems that |
https://gitlab.freedesktop.org/polkit/polkit/-/merge_requests/35 this should be re-fetched from the above MR when forward-porting to updated polkit versions.
commit 5c4d184 Author: Michael Marineau <michael.marineau@coreos.com> Date: Fri Aug 1 14:48:59 2014 -0700 polkit: fix config install paths, use systemd-tmpfiles All configs should be installed to /usr and tmpfiles should be used to create and fix directory permissions instead of the ebuild's postinst.
we don't need to build realmd with introspection support for polkit. Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
it was already the case for ARM64, we just extend it to AMD64. Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
tormath1
force-pushed
the
wrl/polkit-0.119
branch
from
3193012
to
2e62572
Compare
|
@krnowak thanks for reviewing - commits have been rebased / amended / pushed following your comments. |
flatcar-archive/coreos-overlay#1263 made polkit depend on duktape instead of spidermonkey. So we can delete spidermonkey completely.
flatcar-archive/coreos-overlay#1263 made polkit depend on duktape instead of spidermonkey. So we can delete spidermonkey completely.
This PR does two things simultaneously:
Duktape is substantially smaller than Spidermonkey, which both reduces package build time and allows us to ship a smaller image (though I have not determined how much smaller). More importantly, Spidermonkey and its transitive dependencies have been a substantial problem for building arm64 images, and using duktape sidesteps this issue nicely. As such, this PR also resolves the outstanding arm64 polkit issue (ref #863 flatcar/Flatcar#474 flatcar/Flatcar#156).
Of note is that this patchset merge request is marked as WIP and the polkit maintainer has issues with it that are still outstanding. I am going to attempt to resolve them and push this work further along. For the time being, if we ship this duktape'd polkit, we need to be clear publicly that it potentially introduces issues with custom rules.
An option would be to continue depending on Spidermonkey for amd64 and only use duktape for arm64, but a counterpoint is that shipping duktape for both architectures allows for more testing and feedback.
CI 🔵 : http://jenkins.infra.kinvolk.io:8080/job/os/job/manifest/3866/cldsv/